Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix hierarchical otlp props lookup, add Grafana traces check #42763

Closed
wants to merge 1 commit into from
Closed

Fix hierarchical otlp props lookup, add Grafana traces check #42763

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 24 additions & 2 deletions .../io/quarkus/opentelemetry/runtime/config/HierarchicalOTelConnectionConfigInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package io.quarkus.opentelemetry.runtime.config;

import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
import java.util.function.Function;
Expand Down Expand Up @@ -46,7 +46,7 @@ public HierarchicalOTelConnectionConfigInterceptor() {

@Override
public Iterator<String> iterateNames(final ConfigSourceInterceptorContext context) {
Set<String> names = new HashSet<>();
Set<String> names = new LinkedHashSet<>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The LinkedHashmap maintains a predictable iteration order... Can't remember why it is used here anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added it now, before it was HashSet ... but the order wasn't the problem., so I guess we can go back to plain HashSet ... ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, let's go back. This has a performance penalty.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A? With all 1M checks, this is the penalty? :-)
At least we know how they are ordered in the first place.
But yeah, np ... we can go back.

Iterator<String> namesIterator = context.iterateNames();
while (namesIterator.hasNext()) {
String name = namesIterator.next();
Expand All @@ -71,6 +71,28 @@ public Iterator<String> iterateNames(final ConfigSourceInterceptorContext contex
return names.iterator();
}

@Override
public ConfigValue getValue(final ConfigSourceInterceptorContext context, final String name) {
ConfigValue configValue = context.proceed(name);
String map = getMapping().apply(name);

if (name.equals(map)) {
return configValue;
}

// not a default
if (configValue != null && configValue.getConfigSourceOrdinal() > Integer.MIN_VALUE) {
return configValue;
}

ConfigValue fallbackValue = context.proceed(map);
if (fallbackValue != null) {
return fallbackValue.withName(name);
}

return configValue;
}

static class MappingFunction implements Function<String, String> {
@Override
public String apply(String name) {
Expand Down
26 changes: 2 additions & 24 deletions ...me/src/main/java/io/quarkus/opentelemetry/runtime/exporter/otlp/OTelExporterRecorder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,43 +310,21 @@ private static Map<String, String> populateTracingExportHttpHeaders(OtlpExporter
}

private URI getTracesUri(OtlpExporterRuntimeConfig exporterRuntimeConfig) {
String endpoint = resolveTraceEndpoint(exporterRuntimeConfig);
String endpoint = exporterRuntimeConfig.traces().endpoint().orElse(DEFAULT_GRPC_BASE_URI);
if (endpoint.isEmpty()) {
return null;
}
return ExporterBuilderUtil.validateEndpoint(endpoint);
}

private URI getMetricsUri(OtlpExporterRuntimeConfig exporterRuntimeConfig) {
String endpoint = resolveTraceEndpoint(exporterRuntimeConfig);
String endpoint = exporterRuntimeConfig.metrics().endpoint().orElse(DEFAULT_GRPC_BASE_URI);
if (endpoint.isEmpty()) {
return null;
}
return ExporterBuilderUtil.validateEndpoint(endpoint);
}

static String resolveTraceEndpoint(final OtlpExporterRuntimeConfig runtimeConfig) {
String endpoint = runtimeConfig.traces().endpoint()
.filter(OTelExporterRecorder::excludeDefaultEndpoint)
.orElse(runtimeConfig.endpoint()
.filter(OTelExporterRecorder::excludeDefaultEndpoint)
.orElse(DEFAULT_GRPC_BASE_URI));
return endpoint.trim();
}

static String resolveMetricEndpoint(final OtlpExporterRuntimeConfig runtimeConfig) {
String endpoint = runtimeConfig.metrics().endpoint()
.filter(OTelExporterRecorder::excludeDefaultEndpoint)
.orElse(runtimeConfig.endpoint()
.filter(OTelExporterRecorder::excludeDefaultEndpoint)
.orElse(DEFAULT_GRPC_BASE_URI));
return endpoint.trim();
}

private static boolean excludeDefaultEndpoint(String endpoint) {
return !DEFAULT_GRPC_BASE_URI.equals(endpoint);
}

static class HttpClientOptionsConsumer implements Consumer<HttpClientOptions> {
private final OtlpExporterConfig config;
private final URI baseUri;
Expand Down
42 changes: 32 additions & 10 deletions ...rc/test/java/io/quarkus/opentelemetry/runtime/exporter/otlp/OtlpExporterProviderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,82 +17,104 @@

class OtlpExporterProviderTest {

static String resolveTraceEndpoint(final OtlpExporterRuntimeConfig runtimeConfig) {
String endpoint = runtimeConfig.traces().endpoint()
.filter(OtlpExporterProviderTest::excludeDefaultEndpoint)
.orElse(runtimeConfig.endpoint()
.filter(OtlpExporterProviderTest::excludeDefaultEndpoint)
.orElse(DEFAULT_GRPC_BASE_URI));
return endpoint.trim();
}

static String resolveMetricEndpoint(final OtlpExporterRuntimeConfig runtimeConfig) {
String endpoint = runtimeConfig.metrics().endpoint()
.filter(OtlpExporterProviderTest::excludeDefaultEndpoint)
.orElse(runtimeConfig.endpoint()
.filter(OtlpExporterProviderTest::excludeDefaultEndpoint)
.orElse(DEFAULT_GRPC_BASE_URI));
return endpoint.trim();
}

private static boolean excludeDefaultEndpoint(String endpoint) {
return !DEFAULT_GRPC_BASE_URI.equals(endpoint);
}

@Test
public void resolveTraceEndpoint_newWins() {
assertEquals("http://localhost:2222/",
OTelExporterRecorder.resolveTraceEndpoint(createOtlpExporterRuntimeConfig(
resolveTraceEndpoint(createOtlpExporterRuntimeConfig(
"http://localhost:1111/",
"http://localhost:2222/")));
}

@Test
public void resolveTraceEndpoint_globalWins() {
assertEquals("http://localhost:1111/",
OTelExporterRecorder.resolveTraceEndpoint(createOtlpExporterRuntimeConfig(
resolveTraceEndpoint(createOtlpExporterRuntimeConfig(
"http://localhost:1111/",
DEFAULT_GRPC_BASE_URI)));
}

@Test
public void resolveTraceEndpoint_legacyTraceWins() {
assertEquals("http://localhost:2222/",
OTelExporterRecorder.resolveTraceEndpoint(createOtlpExporterRuntimeConfig(
resolveTraceEndpoint(createOtlpExporterRuntimeConfig(
DEFAULT_GRPC_BASE_URI,
"http://localhost:2222/")));
}

@Test
public void resolveTraceEndpoint_legacyGlobalWins() {
assertEquals(DEFAULT_GRPC_BASE_URI,
OTelExporterRecorder.resolveTraceEndpoint(createOtlpExporterRuntimeConfig(
resolveTraceEndpoint(createOtlpExporterRuntimeConfig(
DEFAULT_GRPC_BASE_URI,
null)));
}

@Test
public void resolveTraceEndpoint_testIsSet() {
assertEquals(DEFAULT_GRPC_BASE_URI,
OTelExporterRecorder.resolveTraceEndpoint(createOtlpExporterRuntimeConfig(
resolveTraceEndpoint(createOtlpExporterRuntimeConfig(
null,
null)));
}

@Test
public void resolveMetricEndpoint_newWins() {
assertEquals("http://localhost:2222/",
OTelExporterRecorder.resolveMetricEndpoint(createOtlpExporterRuntimeConfig(
resolveMetricEndpoint(createOtlpExporterRuntimeConfig(
"http://localhost:1111/",
"http://localhost:2222/")));
}

@Test
public void resolveMetricEndpoint_globalWins() {
assertEquals("http://localhost:1111/",
OTelExporterRecorder.resolveMetricEndpoint(createOtlpExporterRuntimeConfig(
resolveMetricEndpoint(createOtlpExporterRuntimeConfig(
"http://localhost:1111/",
DEFAULT_GRPC_BASE_URI)));
}

@Test
public void resolveMetricEndpoint_legacyTraceWins() {
assertEquals("http://localhost:2222/",
OTelExporterRecorder.resolveMetricEndpoint(createOtlpExporterRuntimeConfig(
resolveMetricEndpoint(createOtlpExporterRuntimeConfig(
DEFAULT_GRPC_BASE_URI,
"http://localhost:2222/")));
}

@Test
public void resolveMetricEndpoint_legacyGlobalWins() {
assertEquals(DEFAULT_GRPC_BASE_URI,
OTelExporterRecorder.resolveMetricEndpoint(createOtlpExporterRuntimeConfig(
resolveMetricEndpoint(createOtlpExporterRuntimeConfig(
DEFAULT_GRPC_BASE_URI,
null)));
}

@Test
public void resolveMetricEndpoint_testIsSet() {
assertEquals(DEFAULT_GRPC_BASE_URI,
OTelExporterRecorder.resolveMetricEndpoint(createOtlpExporterRuntimeConfig(
resolveMetricEndpoint(createOtlpExporterRuntimeConfig(
null,
null)));
}
Expand Down
3 changes: 0 additions & 3 deletions integration-tests/observability-lgtm/src/main/resources/application.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,9 @@ quarkus.micrometer.export.otlp.enabled=true
quarkus.micrometer.export.otlp.publish=true
quarkus.micrometer.export.otlp.step=PT5S
quarkus.micrometer.export.otlp.default-registry=true
%dev.quarkus.micrometer.export.otlp.url=http://${quarkus.otel-collector.url}/v1/metrics
%prod.quarkus.micrometer.export.otlp.url=http://localhost:4318/v1/metrics

#opentelemetry
quarkus.otel.exporter.otlp.traces.protocol=http/protobuf
%dev.quarkus.otel.exporter.otlp.traces.endpoint=http://${quarkus.otel-collector.url}
%prod.quarkus.otel.exporter.otlp.traces.endpoint=http://localhost:4318

#quarkus.observability.lgtm.image-name=grafana/otel-lgtm
3 changes: 3 additions & 0 deletions ...on-tests/observability-lgtm/src/test/java/io/quarkus/observability/test/LgtmTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ public void testTracing() {
Awaitility.await().atMost(61, TimeUnit.SECONDS).until(
() -> client.query("xvalue_X"),
result -> !result.data.result.isEmpty());
Awaitility.await().atMost(61, TimeUnit.SECONDS).until(
() -> client.traces("quarkus-integration-test-observability-lgtm", 20, 3),
result -> !result.traces.isEmpty());
}

}
19 changes: 19 additions & 0 deletions ...observability-lgtm/src/test/java/io/quarkus/observability/test/support/GrafanaClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,4 +89,23 @@ public QueryResult query(String query) {
});
return ref.get();
}

public TempoResult traces(String service, int limit, int spss) {
AtomicReference<TempoResult> ref = new AtomicReference<>();
String path = "/api/datasources/proxy/uid/tempo/api/search?q=%7Bresource.service.name%3D%22"
+ service + "%22%7D&limit=" + limit + "&spss=" + spss;
handle(
path,
HttpRequest.Builder::GET,
HttpResponse.BodyHandlers.ofString(),
(r, b) -> {
try {
TempoResult result = MAPPER.readValue(b, TempoResult.class);
ref.set(result);
} catch (JsonProcessingException e) {
throw new UncheckedIOException(e);
}
});
return ref.get();
}
}
38 changes: 38 additions & 0 deletions ...s/observability-lgtm/src/test/java/io/quarkus/observability/test/support/TempoResult.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package io.quarkus.observability.test.support;

import java.util.List;
import java.util.Map;

import com.fasterxml.jackson.annotation.JsonIgnoreProperties;

@JsonIgnoreProperties(ignoreUnknown = true)
public class TempoResult {
public List<Map<Object, Object>> traces;
public Metrics metrics;

// getters and setters

@Override
public String toString() {
return "TempoResult{" +
"traces=" + traces +
", metrics=" + metrics +
'}';
}

@JsonIgnoreProperties(ignoreUnknown = true)
public static class Metrics {
public int inspectedBytes;
public int completedJobs;
public int totalJobs;

@Override
public String toString() {
return "Metrics{" +
"inspectedBytes=" + inspectedBytes +
", completedJobs=" + completedJobs +
", totalJobs=" + totalJobs +
'}';
}
}
}
Loading