how to map declarative config properties #14144
Conversation
|
@trask @jack-berg please have a look |
| db-statement-sanitizer: | ||
| enabled: true # already exists - just for reference | ||
| thread_details: | ||
| enabled: true # was "otel.javaagent.add-thread-details" = "true" |
There was a problem hiding this comment.
alternatively: spans: enabled: true
There was a problem hiding this comment.
it's a good question
honestly I wonder how useful this instrumentation is, and whether we should disable it by default in 3.0 @laurit?
I guess my preference would be
- disabled by default
- enabled applies to both spans and logs (but not metrics due to high cardinality)
There was a problem hiding this comment.
I guess we could apply this new semantic also to the spring starter where this feature is added without declarative config in #14117
There was a problem hiding this comment.
The semantic conventions recommend setting these attributes. Comments in open-telemetry/opentelemetry-specification#788 list some potential usages. It is difficult to predict when which attributes could be valuable.
There was a problem hiding this comment.
@trask agreed to check with sem conv
|
@jack-berg I think we could get this unblocked with your feedback 😄 |
|
Strictly speaking we don't need to merge this, as we normally don't have spec PRs in this repo. Eventually, this doc should end up in the docs repo. |
- use sdk style discriminator for logging - use common "enabled" for "default-enabled"
| db_statement_sanitizer: | ||
| enabled: true # already exists - just for reference | ||
| thread_details: | ||
| # disabled by default |
There was a problem hiding this comment.
currently it is enabled by default, it is disabled when running tests
There was a problem hiding this comment.
@trask suggested that is should be off by default: #14144 (comment)
There was a problem hiding this comment.
I thought that this is supposed to represent what the agent currently does, feel free to ignore if it has a different purpose.
There was a problem hiding this comment.
It's supposed to be the spec for declarative configuration.
Mostly what the agent does today - but it's an opportunity to clean things up - that we could otherwise only do with a major version.
| enabled: true # already exists - just for reference | ||
| thread_details: | ||
| # disabled by default | ||
| # enabled applies to both spans and logs (but not metrics due to high cardinality) |
There was a problem hiding this comment.
this setting does not apply to logs, they use a different flag for the thread attributes
There was a problem hiding this comment.
then we should probably add this here as thread_details/logging/enabled
There was a problem hiding this comment.
the current attributes to add thread details to logs are
otel.instrumentation.log4j-appender.experimental-log-attributesotel.instrumentation.logback-appender.experimental-log-attributes
I similar argument could be made where we have different settings for adding baggage to log context:
otel.instrumentation.logback-mdc.add-baggageotel.instrumentation.log4j-context-data.add-baggage
This is a specification PR - implementation issues will be created when this PR is merged.
Fixes #14132