We are seeing examples where Spring Scheduling INTERNAL spans are
created inside of an existing parent span, which creates unnecessary
noise.
And these spans don't necessary make sense as these are not "background
jobs" (since they occur inside of an existing span).
(see for example
https://github.com/microsoft/ApplicationInsights-Java/issues/2870)
This PR changes Spring Scheduling instrumentation to only instrumenting
repeating jobs, not one-time scheduled jobs (which corresponds to
ScheduledExecutorService behavior where context is not propagated to
runnable)
---------
Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
Hopefully resolves
https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/7597
Without reproducing the issue it is hard to tell whether this will help.
Another issue that could arise is that we add our metrics class in
`metric.reporters` property which will probably break if this
configuration is used to build consumer or producer after deserializing
as our classes don't seem to be available there. If this fails we'll
need to ask the issue reporter for instructions how to reproduce and
find a different strategy for fixing this.
Fix#7729
This PR adopts Azure SDK tracing API changes from the latest release
(azure-core 1.36.0, azure-core-tracing-opentelemetry 1.0.0-beta.32)
The API changes are not breaking (1.19 instrumentation is still
compatible), but the new instrumentation is slightly more performant and
supports new features. We are also going to break compatibility with
1.19 instrumentation at some point (in 6-12 months).
We now have 3 versions for azure-sdk. We still have about 10% of users
on versions [1.14-1.19), but it's declining and I'll be happy to remove
1.14 in the next few months if this trend continues.
---------
Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
Resolves
https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/7597
I wasn't able to reproduce this. Figuring out how to run beam, flink and
kafka together feels like too much effort. Without reproducing it is too
hard to tell why the configuration is serialized, but my hunch is that
it is enough to ensure that the configuration can be serialized.
the `getMdcCopy` test is omitted as it is only available in
jboss-logmanager 1.3+ and the test depends on jboss-logmanager 1.1
---------
Co-authored-by: opentelemetrybot <107717825+opentelemetrybot@users.noreply.github.com>
…n-api-semconv
We're already doing that for `SpanNameExtractor`, `OperationMetrics`,
`ContextCustomizer`, etc., so I figured we should do the same for
`AttributesExtractor` implementation. Also, none of the implementations
have any additional public surface - aside from the builder/factory
method users can just simply use the interface everywhere.
While using Ratpack Handlers, the context is added by the
`OpenTelemetryServerHandler`
While using Ratpack Services, we might need to add manually the OT
Context into the Ratpack Execution and try to get it from the Execution
in the Ratpack `HttpClient`
This is another follow-up from #7616. This makes the test options class
immutable and uses `@AutoValue` and `@AutoValue.Builder`. As a result, a
bunch of the configuration/setup code for these said options now flings
around a builder instance. This isn't great, but I think it's an
incremental improvement that can be seen in the `@BeforeAll
AbstractHttpClientTest.setupOptions()` method, where the immutable
options are (finally) instantiated.
I decided splitting changes on different PRs due to there are a lot of
lines of code in tests here and it should simplify review process.
Another option is I can add additional commit to this PR with conversion
of other groovy files.
As part of discussions #7616, the idea of trying to do a more piecemeal
approach came up. A reasonable ask.
This is the first step in refactoring the http client tests. It factors
out the `HttpClientResult` inner class of the `AbstractHttpClientTest`
so that this can be reused by new test framework later. It also factors
the relevant abstract methods in the abstract class to a new type
adapter, which will also be reused.
Co-authored-by: Mateusz Rzeszutek <mrzeszutek@splunk.com>
This PR includes updates to the SQLSanitizer, DbClientSpanNameExtractor
and SqlStatementInfo to name spans according to procedure name for CALL
statements. The updates to the naming logic are in the SqlSanitizer and
table has been renamed to identifier as using the table variable for the
procedure name would not be idiomatic. SqlStatementInfo has been updated
so that the db.sql.table attribute is not included for procedures.
Solves. #6991
This PR implements the request portion of the new gRPC metadata
instrumentation spec:
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/rpc.md#grpc-request-and-response-metadata
The changes include:
- new CommonConfig entry for desired gRPC metadata values:
'otel.instrumentation.grpc.capture-metadata.request'
(Similar to http headers)
- setting the desired metadata values in GrpcTelemetry
- new property in GrpcAttributesExtractor that holds a reference to the
GrpcRpcAttributesGetter
- new property in GrpcAttributesExtractor that stores the desired values
so it can iterate them and extract each one from the request
- inject the GrpcRpcAttributesGetter to GrpcAttributesExtractor (in
GrpcTelemetryBuilder)
- logic in GrpcRpcAttributesGetter to safely extract the gRPC metadata
value
- A new test in GrpcTest that makes sure that when a certain metadata
key name is inserted, it also ends up in the span attributes
** Doesn't take care of the response because gRPC response is not
implemented in java-instrumentation yet. (This is absolutely necessary
but out of scope for this PR)
** "metadataValue" is only implemented inside GrpcRpcAttributesGetter
and not in RpcAttributesGetter to avoid providing implementations for
every RpcAttributesGetter in the repo as this PR only focuses on gRPC.
Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
Let's keep close to the SDK repo config.
I reverted some of the changes, only left those that I think make sense
anyway (e.g. comparing enums with `==`)