Implement new SemConv exporter health metrics #7265
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hey @JonasKunz give me a heads up when you think this is ready for review. Happy to start with a high level review while its still in draft! |
|
@jack-berg a high level review would be appreciated! I'm happy with the overall structure and would mostly polish from now on, so it would be great to get your feedback before I polish things we'll end up changing anyway. I've updated the PR comment to give an introduction on how I envision the feature implementation in general. |
# Conflicts: # exporters/common/src/main/java/io/opentelemetry/exporter/internal/ExporterMetrics.java # exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/metrics/OtlpHttpMetricExporterBuilder.java # exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterBuilder.java
jack-berg
reviewed
Apr 30, 2025
| private static final AttributeKey<Boolean> ATTRIBUTE_KEY_SUCCESS = booleanKey("success"); | ||
| /** | ||
| * This class is internal and is hence not for public use. Its APIs are unstable and can change at | ||
| * any time. |
There was a problem hiding this comment.
Out of curiosity did the build force this comment to pass?
There was a problem hiding this comment.
Yes:
> Compilation failed; see the compiler output below.
/Users/jonas/git/otel/opentelemetry-java/exporters/common/src/main/java/io/opentelemetry/exporter/internal/ExporterMetrics.java:29: warning: [OtelInternalJavadoc] This public internal class doesn't end with any of the applicable javadoc disclaimers: "This class is internal and is hence not for public use. Its APIs are unstable and can change at any time.", or "This class is internal and experimental. Its APIs are unstable and can change at any time. Its APIs (or a version of them) may be promoted to the public stable API in the future, but no guarantees are made."
public enum Signal {
^
(see https://errorprone.info/bugpattern/OtelInternalJavadoc)
1 error
1 warning
Is that not intentional?
sdk/common/src/main/java/io/opentelemetry/sdk/common/HealthMetricLevel.java
Outdated
Show resolved
Hide resolved
exporters/common/src/main/java/io/opentelemetry/exporter/internal/ExporterMetricsAdapter.java
Outdated
Show resolved
Hide resolved
exporters/zipkin/src/main/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterBuilder.java
Outdated
Show resolved
Hide resolved
exporters/zipkin/src/main/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterBuilder.java
Outdated
Show resolved
Hide resolved
exporters/otlp/all/src/jmh/java/io/opentelemetry/exporter/otlp/trace/OltpExporterBenchmark.java
Outdated
Show resolved
Hide resolved
exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporterBuilder.java
Outdated
Show resolved
Hide resolved
exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporterBuilder.java
Outdated
Show resolved
Hide resolved
exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporter.java
Outdated
Show resolved
Hide resolved
exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporter.java
Outdated
Show resolved
Hide resolved
# Conflicts: # docs/apidiffs/current_vs_latest/opentelemetry-exporter-otlp.txt # docs/apidiffs/current_vs_latest/opentelemetry-exporter-zipkin.txt # docs/apidiffs/current_vs_latest/opentelemetry-sdk-common.txt
jack-berg
approved these changes
May 27, 2025
There was a problem hiding this comment.
There may be opportunities to clean up some things (e.g. its not great that ExporterInstrumentation and ExporterMetrics each have an internal class/interface called Recording), but this is great start as far as I'm concerned. New public API surface area and telemetry output looks good.
A couple more non-blocking comments
exporters/common/src/test/java/io/opentelemetry/exporter/internal/http/HttpExporterTest.java
Show resolved
Hide resolved
exporters/common/src/test/java/io/opentelemetry/exporter/internal/grpc/GrpcExporterTest.java
Show resolved
Hide resolved
exporters/common/src/test/java/io/opentelemetry/exporter/internal/http/FakeHttpResponse.java
Outdated
Show resolved
Hide resolved
...rs/common/src/main/java/io/opentelemetry/exporter/internal/metrics/ServerAttributesUtil.java
Outdated
Show resolved
Hide resolved
.../java/io/opentelemetry/exporter/otlp/testing/internal/AbstractGrpcTelemetryExporterTest.java
Outdated
Show resolved
Hide resolved
…nal/grpc/GrpcExporterTest.java Co-authored-by: jack-berg <34418638+jack-berg@users.noreply.github.com>
…nal/http/HttpExporterTest.java Co-authored-by: jack-berg <34418638+jack-berg@users.noreply.github.com>
jkwatson
reviewed
May 28, 2025
sdk/common/src/main/java/io/opentelemetry/sdk/internal/Signal.java
Outdated
Show resolved
Hide resolved
jkwatson
reviewed
May 28, 2025
sdk/common/src/main/java/io/opentelemetry/sdk/internal/StandardComponentId.java
Outdated
Show resolved
Hide resolved
trask
reviewed
May 29, 2025
sdk/common/src/main/java/io/opentelemetry/sdk/common/InternalTelemetryVersion.java
Outdated
Show resolved
Hide resolved
jack-berg
approved these changes
Jun 3, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements the new, experimental exporter health metrics defined in semantic conventions. The other metrics will be added in follow-up PRs.
We already have some experimental health metrics in the SDK, which I intend to eventually replace with the new ones from semantic conventions. I'm envisioning the replacement process as follows:
on,off,legacyand maybe in the futureextendedfor opt-in attributes / metricslegacytoon, this will be a breaking change