-
Notifications
You must be signed in to change notification settings - Fork 1k
Emit schema url from HTTP instrumentations #15144
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.
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
Conversation
...try/instrumentation/api/incubator/builder/internal/DefaultHttpClientInstrumenterBuilder.java
Show resolved
Hide resolved
The test framework was not copying the schema URL from the protobuf ScopeSpans to the InstrumentationScopeInfo when converting spans, causing all schema URL assertions to fail even though spans were correctly emitting schema URLs.
The instrumenterBuilder() method in DefaultHttpClientInstrumenterBuilder was creating new Instrumenter builders without setting the schema URL. This affected connection-level instrumentations (e.g., Netty connection spans for error cases) which use this method to create their instrumenters. Fixes failing tests in finagle-http-23.11 where connection error spans were missing the schema URL.
The TelemetryConverter now correctly preserves schema URLs from protobuf data. The MeterTest files create meters with schema URL 'http://schema.org', so the test expectations need to be updated to match.
| .addAttributesExtractors(additionalExtractors) | ||
| .addOperationMetrics(HttpClientMetrics.get()); | ||
| .addOperationMetrics(HttpClientMetrics.get()) | ||
| .setSchemaUrl(SchemaUrls.V1_37_0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does setting schema url pose any additional restrictions to what we can do with these instrumentations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can still emit additional attributes if that what you're asking?
Related to #12846