-
Notifications
You must be signed in to change notification settings - Fork 829
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
Extract ConfigProperties
to a separate module so that it can be used in instrumentation-api
#4563
Comments
If we remove |
Hmm, if we change the package name (which now contains words |
Yeah if we go ahead with the extraction we'd definitely need the bridging |
Couple of options I see:
I think option 2 is probably best long term. In addition to the agent being able to benefit from it, there are various places in the SDK that would benefit from reading from |
option 1 is not viable, since the Java Module System precludes 2 jars from containing classes in the same package (in the same classloader). |
Option 3 is also not viable --
One more downside is that most stable SPIs (like |
we discussed in office hours and @jkwatson had a good thought that in the future we'll probably want a richer configuration data structure (e.g. for yaml-based config) that would cover both sdk and instrumentation configuration, and that would be a good time to introduce a new config API. and so in the meantime, the current thought is not to touch the autoconfigure ConfigProperties. I'll open a new issue in the instrumentation repo to discuss next steps there, but feel free to post here if you think we should still consider changes in autoconfigure ConfigProperties. |
I had also thought of this, my idea was to rename |
I think this can be closed now; we're going to have a separate, internal (for now) agent-only config that simply forwards all calls to |
Is your feature request related to a problem? Please describe.
We'd like to use
ConfigProperties
in the instrumentation code too, and get rid of the duplicateConfig
functionality. As it is now,ConfigProperties
lives in theautoconfigure-spi
module, which we cannot use ininstrumentation-api
because it has SDK dependencies.Describe the solution you'd like
We could extract
ConfigProperties
to a separateopentelemetry-config
module that does not depend on anything else; and use it both in the SDK autoconfigure andinstrumentation-api
(and all instrumentations too).Describe alternatives you've considered
Moving
ConfigProperties
toopentelemetry-api
is a no-go; we don't really want to have a duplicateConfig
either.Additional context
Related to open-telemetry/opentelemetry-java-instrumentation#6202
The text was updated successfully, but these errors were encountered: