-
Notifications
You must be signed in to change notification settings - Fork 1k
Move Akka and Armeria virtual fields to helpers #13604
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
Merged
laurit
merged 8 commits into
open-telemetry:main
from
JonasKunz:extract-akka-virtual-fields
Apr 1, 2025
Merged
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
f33244d
Move akka virtual fields to static fields
JonasKunz c9a7257
Add armeria as simple example
JonasKunz b3eee08
Add remaining akka virtual field usages
JonasKunz e02de3a
Fix rawtype usage
JonasKunz 0f1460d
Add utility class for grpc authority storage
JonasKunz b88b9fe
Revert changes to OtelPrivateConstructorForUtilityClass
JonasKunz db122af
Fix compilation warnings and spotless
JonasKunz 1c21bf7
encapsulate virtual field use in util class
laurit File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
22 changes: 22 additions & 0 deletions
22
...ent/src/main/java/io/opentelemetry/javaagent/instrumentation/akkaactor/VirtualFields.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| /* | ||
| * Copyright The OpenTelemetry Authors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package io.opentelemetry.javaagent.instrumentation.akkaactor; | ||
|
|
||
| import akka.dispatch.Envelope; | ||
| import akka.dispatch.sysmsg.SystemMessage; | ||
| import io.opentelemetry.instrumentation.api.util.VirtualField; | ||
| import io.opentelemetry.javaagent.bootstrap.executors.PropagatedContext; | ||
|
|
||
| public class VirtualFields { | ||
|
|
||
| private VirtualFields() {} | ||
|
|
||
| public static final VirtualField<Envelope, PropagatedContext> ENVELOPE_PROPAGATED_CONTEXT = | ||
| VirtualField.find(Envelope.class, PropagatedContext.class); | ||
| public static final VirtualField<SystemMessage, PropagatedContext> | ||
| SYSTEM_MESSAGE_PROPAGATED_CONTEXT = | ||
| VirtualField.find(SystemMessage.class, PropagatedContext.class); | ||
| } |
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
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
24 changes: 24 additions & 0 deletions
24
...ent/src/main/java/io/opentelemetry/javaagent/instrumentation/akkaactor/VirtualFields.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| /* | ||
| * Copyright The OpenTelemetry Authors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package io.opentelemetry.javaagent.instrumentation.akkaactor; | ||
|
|
||
| import akka.dispatch.forkjoin.ForkJoinTask; | ||
| import io.opentelemetry.instrumentation.api.util.VirtualField; | ||
| import io.opentelemetry.javaagent.bootstrap.executors.PropagatedContext; | ||
| import java.util.concurrent.Callable; | ||
|
|
||
| public class VirtualFields { | ||
|
|
||
| private VirtualFields() {} | ||
|
|
||
| public static final VirtualField<ForkJoinTask<?>, PropagatedContext> | ||
| FORK_JOIN_TASK_PROPAGATED_CONTEXT = | ||
| VirtualField.find(ForkJoinTask.class, PropagatedContext.class); | ||
| public static final VirtualField<Runnable, PropagatedContext> RUNNABLE_PROPAGATED_CONTEXT = | ||
| VirtualField.find(Runnable.class, PropagatedContext.class); | ||
| public static final VirtualField<Callable<?>, PropagatedContext> CALLABLE_PROPAGATED_CONTEXT = | ||
| VirtualField.find(Callable.class, PropagatedContext.class); | ||
| } |
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
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
17 changes: 17 additions & 0 deletions
17
.../java/io/opentelemetry/javaagent/instrumentation/akkahttp/server/route/VirtualFields.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| /* | ||
| * Copyright The OpenTelemetry Authors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package io.opentelemetry.javaagent.instrumentation.akkahttp.server.route; | ||
|
|
||
| import akka.http.scaladsl.server.PathMatcher; | ||
| import io.opentelemetry.instrumentation.api.util.VirtualField; | ||
|
|
||
| public class VirtualFields { | ||
|
|
||
| private VirtualFields() {} | ||
|
|
||
| public static final VirtualField<PathMatcher<?>, String> PATH_MATCHER_ROUTE = | ||
| VirtualField.find(PathMatcher.class, String.class); | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,6 +34,11 @@ public void transform(TypeTransformer transformer) { | |
| @SuppressWarnings("unused") | ||
| public static class ConstructorAdvice { | ||
|
|
||
| public static class VirtualFields { | ||
|
||
| public static final VirtualField<ServerCall<?, ?>, String> SERVER_CALL_AUTHORITY = | ||
| VirtualField.find(ServerCall.class, String.class); | ||
| } | ||
|
|
||
| @Advice.OnMethodExit(suppress = Throwable.class) | ||
| public static void onExit( | ||
| @Advice.This ServerCall<?, ?> serverCall, | ||
|
|
@@ -43,7 +48,7 @@ public static void onExit( | |
| // ArmeriaServerCall does not implement getAuthority. We will store the value for authority | ||
| // header as virtual field, this field is read in grpc instrumentation in | ||
| // TracingServerInterceptor | ||
| VirtualField.find(ServerCall.class, String.class).set(serverCall, authority); | ||
| VirtualFields.SERVER_CALL_AUTHORITY.set(serverCall, authority); | ||
| } | ||
| } | ||
| } | ||
|
|
||
Oops, something went wrong.
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.
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.
I'd say this is probably not worth it. Instead of creating the nested
VirtualFieldsclass in advice you are better off encapsulating the virtual field access in a util class that internally uses the virtual field. For example in case ofArmeriaServerCallInstrumentationI should have really added an util class in https://github.com/open-telemetry/opentelemetry-java-instrumentation/tree/main/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/internal and done the virtual field access there. It is easier to search from ide where the util class is used instead of searching for something likeVirtualField.find(ServerCall.class, String.class). If creating a new util class for the virtual fields feels like an overkill we could add the logic to theSingletonsclass. I think the separateVirtualFieldsclass is fineish in the akka instrumentation for the task context propagation.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.
Makes sense, fixed in 0f1460d