Rename autoconfigure modules #4779
Conversation
trask
requested review from
anuraaga,
iNikem,
jkwatson,
laurit,
mateuszrzeszutek,
pavolloffay and
tylerbenson
as code owners
| @@ -264,7 +264,7 @@ include(":instrumentation:liberty:liberty-dispatcher:javaagent") | |||
| include(":instrumentation:log4j:log4j-1.2:javaagent") | |||
| include(":instrumentation:log4j:log4j-2.7:javaagent") | |||
| include(":instrumentation:log4j:log4j-2.13.2:javaagent") | |||
| include(":instrumentation:log4j:log4j-2.13.2:library") | |||
| include(":instrumentation:log4j:log4j-2.13.2:library-autoconfigure") | |||
There was a problem hiding this comment.
I think so but just want to confirm we're comfortable with this naming for libraries that don't use GlobalOpenTelemetry.
There was a problem hiding this comment.
Also - ok for followup but I guess we do need to split the appender out sooner than later for this one
There was a problem hiding this comment.
Good point. Maybe we define autoconfigure as any module that is enabled automatically just by being on the class path.
There was a problem hiding this comment.
I think the -autoconfigure naming is a nice indication to users that there's nothing they have to do to register/configure it, so seems reasonably worth it even if there's would never be a non-autoconfigure module.
There was a problem hiding this comment.
Also - ok for followup but I guess we do need to split the appender out sooner than later for this one
oh yes, created #4780 and set milestone for 1.10.0 to limit breakage
There was a problem hiding this comment.
Just seeing this now. This case is interesting because parts of this module are enabled automatically (i.e. the context provider) while other have to be manually enabled (i.e. the log4j2 log appender for the log sdk).
Thoughts?
There was a problem hiding this comment.
I think we would have the appender only in library and the context provider in auto, so basically split in half
Resolves #4778 (and needed for #4771)