WFLY-20296 - Add support for more OpenTelemetry Config options

[jasondlee]

https://issues.redhat.com/browse/WFLY-20296

Add analysis document

Resolves #683

[bstansberry]

Please discuss with the security team how this kind of thing should be represented; i.e. if we should provide deeper integration with existing Elytron services.

[darranl]

What type of data would attributes for these take?

In general we are trying to centralise configuration related to SSLContexts into the Elytron subsystem so users and administrators don't need to learn how to confiure TLS related resources independently on each separate subsystem,

[jasondlee]

The value passed to the underlying library is the full path to the certificates (and keys) used by the OpenTelemetry Collector to configure TLS on its end. I guess it's possible that we switch the perspective here: rather than pointing WF to what the Collector is using to configure itself, we point the Collector to what WF is using to configure itself?

[darranl]

I am not sure if I am following the inversion.

Where you say OpenTelemetry Collector I assume you are talking about some remote process we are establishing a connection to?

[jasondlee]

Yes. It's a process that runs in another process (or even data center). In my head, the plan is currently to point the OpenTelemetry subsystem at the certificate and key files known to and use by the OpenTelemetry Collector. What I mean by the inversion is that rather than point from inside the app servers config (and FS area) to files owned/managed by the Collector, that could instead point the Collector to cert/key files owned and managed by WildFly. Perhaps I was being too cute. :P I was just thinking "out loud" about how a WildFly SSL context might used to configure an external service. Ultimately, it's going to come down to each system sharing one or more files, regardless of who "owns" those files.

[TomasHofman]

Out of curiosity, it's not clear to me how exactly it affects the developer to rely on MP Config to configure opentelemetry.

My understanding is that the MP Config way simply means that I would e.g. define a system property via -Dotel.service.name=myname (tried and it works), or I could create "permanent" system property in standalone.xml. Perhaps I can also package an application.properties file into my deployment (haven't tried that).

Nothing else is needed on the application side (no special dependency, etc). Am I correct? The "external library" is a detail not visible to the app developer.

[TomasHofman]

Maybe if I wanted to configure via programmatic API, I would need to add a dependency to the app?

[jasondlee]

I more had in mind the server administrator, as the MP Config extension/subsystem would have to be enabled to make it work. I'll try to clean up that wording there.

[TomasHofman]

OK, I get what you meant now.

[bstansberry]

+1 to focusing on an administrator.

Beyond not 'depending on a library' (or setting system properties) in terms of a programming API, someone using this is depending on us for a robust, long-term-compatible API. They are depending on us to insulate them from underlying changes in OTel. If OTel renames one of their props, or whatever, in a future release, that is our problem, not the user's problem. They count on us to shield them from such things. That is what the WildFly management API does for people who use the many libraries we integrate.

So, think very hard about what you expose, as you'll be living with it for a long time. :-)

[TomasHofman]

You probably have reasons not to do it that way, but I will ask :):

Was it considered to introduce a "properties" section in the opentelemetry config model, which would allow users to specify otel properties directly as they are in the otel spec (same keys & values), rather than introducing custom named attributes?

The advantages would be:

• the administrator would not need to investigate how Wildfly attributes map to OTel properties,
• it would (maybe?) allow to configure full set of OTel properties, without extensive changes to the management model,
• it might be more accommodating to potential new attributes added in future.

[jasondlee]

You probably have reasons not to do it that way, but I will ask :):

Was it considered to introduce a "properties" section in the opentelemetry config model, which would allow users to specify otel properties directly as they are in the otel spec (same keys & values), rather than introducing custom named attributes?

I have not thought about that, to be honest, but, having done so a bit now, I'm not sure I'm real excited about that. You're probably right the current approach puts some onus on the admin to map attributes to properties, but I hope the attribute names make it clear enough what they do so the admin doesn't need to know about the mapping. I think more importantly, depending too heavily on an assortment of properties defined elsewhere makes the management interface vague and opaque. Using clearly defined management model attributes for clarity coupled with the properties-based approach via MP Config (or sysprops) gives us a nice middle ground, albeit at the expense of some complexity (like this PR) in adding new, explicit attribute support. I can be persuaded otherwise, but I think that's my take. :)

[bstansberry]

s/as defaults/as initial defaults/g

Once we set a default, it's our default. It's no longer determined based on what some outside party decides. Changing it is a semi-breaking or breaking change, depending on what the attribute configures. (We can change accept that and change the default but we should not promise that we will always do what some outside party said.)

[bstansberry]

Sounds like a -1 from me.

If it's 'experimental' to OTel it should not be in the WF management API, except in it's own low-stability-variant of the management API.

[bstansberry]

Re 'pending discussion with Elytron' -- yes, for sure. A long term strategy for security configuration is a blocker for having this at community stability.

To me one of the value adds of having a subsytem is it does proper cross-subsystem integration for key concerns like managing TLS.

[jasondlee]

I am working on this implementation in consultation with Elytron at this very moment.

[bstansberry]

Does no value mean no compression?

[bstansberry]

What's the plan for when we decide to support other exporters?

I can imagine one; I just want to make sure you have too, as you'll be living with this for many years. :-)

Same applies to Metrics|Enabled and Logs|Enabled

[bstansberry]

The textual description of any such attribute must explicitly declare that if MicroProfile Telemetry is used the config property 'foo' takes precedence.

I hope if MP Tel is not used that system properties are not preferred to what is set via the management API. If that's not the case let's have a zulip or dev list discussion.

[bstansberry]

I'm not clear on the 'non-requirement' nature of this item.

[bstansberry]

For sure it's applicable. Needs a statement, even if it's just that the CLI and HAL will just work with no changes on their side.