previously we were recreating this on each invocation

Indeed by adding the call to GlobalOpenTelemetry.getOrNoop() it will solve the issue on #16506

But now I wonder what would happen to the 'baggageEnabled' flag if this class is called before the GlobalOpenTelemety is propperly configured.

static final boolean baggageEnabled = DeclarativeConfigUtil.getInstrumentationConfig( GlobalOpenTelemetry.getOrNoop(), "log4j_context_data") .getBoolean( "add_baggage", ConfigPropertiesUtil.getBoolean( "otel.instrumentation.log4j-context-data.add-baggage", false));

This code is static and the baggageEnabled is final, so I assume it will hold the value it gets the first time the class is loaded. In that case, if the GlobalOpenTelemetry is not initialised yet, the NoOp implementation will be returned and this boolean will hold whatever the NoOp provides, ignoring any user configuration. (this are assumptions, I would have to try the code to make sure what I'm saying is correct).

If I was implementing I would try an approach like that:

1 - don't make this static, so it will not initialize when the class is loaded, as we have no control when the Log4j will load the plugins (before or after the GlobalTelemetry is loaded)

2 - Create a method to check ifBaggageIsEnabled() and inside the method implement a logic to check
2.1 - Create a Boolean and keep it as NULL as a instance attribute (not static);
2.2 - check if GlobalTelemetry.isSet(), if so, execute the logic you have implemented now to obtain the correct value for the add-baggage property and set it in the Boolean; If it's not, return a default value (probably FALSE).
2.2 - Based in the logic Boolean != null, return the obtained value.

I would say that way we can be safe and independent of initialisation/loading order of our classes (log4j plugins) and the user initialisation of GlobalTelemetry.

What do you think?

The thing is that when using manual instrumentation global open telemetry instance is often not set, in fact it is recommended not to rely on it. The intended use of GlobalOpenTelemetry is to get access to the OpenTelemetry instance configured by the agent. Because of that when using manual instrumentation we can't really guarantee that the declarative configuration can be read at all here (system properties can be read though). To make the declarative configuration work we may need to introduce something like

Humm I see. Maybe I interpreted the docs wrongly, my bad.

TLDR; I'm good with this implementation. I would just like to have this documented somewhere (and I can help on documenting it if needed) so using these classes/plugins would be more straighforward.

So now after your comment, thinking again about it, the ideal scenario would be, for a 'non agent' approach:

• I would have my own Singleton (MyOpenTelemetry) to manage my own openTelmetry instance (to do not rely on GlobalOpenTelemetry).
• In my 'MyOpenTelemetry' I would check if the agente set up some configuration or if I need to setup my own from scratch.
• If I need to get an reference to my application openTelemetry I should use MyOpenTelemetry.get() (or whatever method I create) instead of calling GlobalOpenTelemetry.getOrNoOp()/get() methods.

If what I wrote now is correct, so, I think I understood better the intent of the GlobalTelemetry, thanks for that.

Still, about the problem at hand, with this implementation, what would happen with the baggage-allowed flag if I have a manual instrumentation (not relying on agents) to configure my application? What would be the value for the baggage flag? Would it be mandatory to use a .properties file to configure this flag on a reliable way?

Also I understood that to make the declarative (code based) configuration we would have to implement the install()-like method we have for the appenders. And I believe it's not in the plans for now, right? :)

Either way I think it's fine, I would just like to have this documented somewhere (and I can help on documenting it if needed) so using these classes would be more intuitive.

As I mentioned in the #16506 I end up here because I'm probing how to use the Otel + Log4j integration. I'm trying to use it in a production use case and I'm not very confident yet to do it, so helping to document and troubleshoot would also help myself. ;)

If you rely on having the OpenTelemetry instance available from central location then I think you might as well use the GlobalOpenTelemetry instead of building your own singleton if that is easier for you.
My understanding is that the reasoning behind not advising to use the GlobalOpenTelemetry was related to it making it difficult to reason about initialization order. Like here ideally the global instance would be set before log4j initializes, but it isn't obvious. The default behavior of GlobalOpenTelemetry used to be (this has changed, but can be enabled with a flag) that when the global instance wasn't set it created an autoconfigured instance. This could result in users thinking that they are using their manually configuring OpenTelemetry instance but actually using the autoconfigured one. When OpenTelemetry instance is explicitly passed around then that kind of issues shouldn't happen. Though there are obviously cases where explicitly passing the OpenTelemetry instance is not easy like here.

Still, about the problem at hand, with this implementation, what would happen with the baggage-allowed flag if I have a manual instrumentation (not relying on agents) to configure my application? What would be the value for the baggage flag? Would it be mandatory to use a .properties file to configure this flag on a reliable way?

System properties and environment variables should work as described in https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/library-autoconfigure/README.md For declarative configuration you need to set the global instance before log4j is initialized.

Also I understood that to make the declarative (code based) configuration we would have to implement the install()-like method we have for the appenders. And I believe it's not in the plans for now, right? :)

By declarative configuration we mean yaml based configuration that is described in https://opentelemetry.io/docs/languages/sdk-configuration/declarative-configuration/ and https://opentelemetry.io/docs/zero-code/java/agent/declarative-configuration/ The declarative configuration is still fairly new and not everything has been worked out. There are a couple of other instrumentations that face a similar challenge with declarative configuration.

Either way I think it's fine, I would just like to have this documented somewhere (and I can help on documenting it if needed) so using these classes would be more intuitive.

Documentation for this instrumentation is in https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/library-autoconfigure/README.md

Thanks a lot for your time for both fixing the issue and also for sharing your thoughts and giving clarifications on that subject.

Just for the context: Indeed, I work developing platform level code and instrumenting it. With our current platform architecture I need (at least I think I do :) ) a shared instance of 'OpenTelemetry' to generate our traces in our different libraries. We also have a custom logging library, based on Log4j and we use the same instance OpenTelemetry to link our logs and traces. We still rely, somehow, on GlobalTelemetry, but we do have a 'Façade' to hide it from our final users (developers) and prevent them from miss using the GlobalTelemetry.

My current effort is to understand how much of the OpenTelemetry integration with Log4J and logging I can leverage.

Again, thanks a lot for your time on fixing this bug and for both you and Trask to leading the OpenTelemetry java initiatives.