-
Notifications
You must be signed in to change notification settings - Fork 867
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
Make empty agent bridged context equal root context #3869
Conversation
@@ -40,6 +40,11 @@ | |||
private static final Logger logger = LoggerFactory.getLogger(AgentContextStorage.class); | |||
|
|||
public static final AgentContextStorage INSTANCE = new AgentContextStorage(); |
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.
Is it possible to use the extension?
Does it break muzzle?
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.
Yes, it fails muzzle when older api is used because I call ContextStorage.root()
which was just added. Seems like wrapper based approach won't work on older api because Context.root()
just returns ArrayBasedContext.root()
which we would still need to wrap.
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.
Replaced call to ContextStorage.root()
with method handle, @anuraaga could you review.
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.
👍 created #3884 to add tests for the new AgentContext wrapper bridging
// APPLICATION_ROOT is null when this method is called while the static initializer is | ||
// initializing the value of APPLICATION_ROOT field |
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.
this is a little scary
Inside agent
Context.current() == Context.root()
when context is empty. The same holds when using sdk without agent. It seems reasonable to assume that this should also hold when using sdk with agent.