Demosntrate declarative config 0.3 with otel java agent #492
Conversation
javaagent/sdk-config.yaml
Outdated
| dropwizard-metrics: | ||
| enabled: true |
There was a problem hiding this comment.
This instrumentation is not enbabled by default....do we want it enabled here? This also applies to:
- jdbc-datasource
- micrometer
- mybatis
- spring-batch
There was a problem hiding this comment.
it should reflect the default values I think
There was a problem hiding this comment.
I had a change of heart on this. I realized that it would be near a huge effort to go and enumerate all the config options for all the instrumentation, especially since many are buried in the code without documentation. It would also be fragile, causing the config to break when breaking changes happen to the config, eroding user confidence.
Instead, I opted to give a thorough explanation of the mapping rules from system properties to declarative config YAML, and demonstrate the configuration of just a couple of modules as an example, leaving the user to apply the rules to configure additional instrumentation modules.
I also opted to base the sample config file on sdk-migration-config.yaml, which is much more exhaustive and includes helpful explanatory comments.
…y-java-docs into declarative-config-0.3
…mentation rather than an exhaustive list
javaagent/sdk-migration-config.yaml
Outdated
| # # Drop spans where url.path matches the regex /actuator.* (i.e. spring boot actuator endpoints). | ||
| # - action: DROP | ||
| # attribute: url.path | ||
| # pattern: /actuator.* |
There was a problem hiding this comment.
Check out this fun snippet! This is the declarative config solution to open-telemetry/opentelemetry-java-instrumentation#1060
javaagent/Dockerfile
Outdated
|
|
||
| ENTRYPOINT java -jar -javaagent:/opentelemetry-javaagent.jar /app.jar | ||
| ENTRYPOINT java -jar -javaagent:/opentelemetry-javaagent.jar -Dotel.javaagent.extensions=/opentelemetry-javaagent-extension.jar /app.jar |
There was a problem hiding this comment.
Loading the rule based sampler artifact as an agent extension, so it can be referenced in the declarative config file.
There was a problem hiding this comment.
when i was running this locally i had issues with the ordering of -jar -javaagent
| ENTRYPOINT java -jar -javaagent:/opentelemetry-javaagent.jar -Dotel.javaagent.extensions=/opentelemetry-javaagent-extension.jar /app.jar | |
| ENTRYPOINT java -javaagent:/opentelemetry-javaagent.jar -jar -Dotel.javaagent.extensions=/opentelemetry-javaagent-extension.jar /app.jar |
There was a problem hiding this comment.
Weird.. I wonder why I didn't.. I moved -jar all the way to the end, right before /app.jar.
javaagent/sdk-migration-config.yaml
Outdated
| # Uncomment to configure the rule_based_routing sampler from https://github.com/open-telemetry/opentelemetry-java-contrib/tree/main/samplers | ||
| # to drop spans to spring actuator endpoints. | ||
| # Configure the parent_based sampler's root sampler to be rule_based_routing sampler. |
There was a problem hiding this comment.
The thought was to leave the default parent based sampler config from opentelemetry-config, but also show the user how they might change the config to solve this use case.
Backing up a bit, I think we (the java SIG) need to maintain start template(s), based on the opentelemetry-configuration examples, but with the additional properties that java users will often care about. If we agree on that, where should those templates live and what should their contents be? Should this config file serve that purpose, either temporarily or permanently?
There was a problem hiding this comment.
I went ahead and set the root sampler to the rule based routing sampler by default. Called out this difference from the standard sdk-config.yaml template in comments at the top of the file. I figure its such an important use case that its useful to highlight.
javaagent/sdk-migration-config.yaml
Outdated
| # Configure root sampler. | ||
| root: | ||
| # Configure sampler to be always_on. | ||
| always_on: {} | ||
| # Configure remote_parent_sampled sampler. | ||
| remote_parent_sampled: | ||
| # Configure sampler to be always_on. | ||
| always_on: {} | ||
| # Configure remote_parent_not_sampled sampler. | ||
| remote_parent_not_sampled: | ||
| # Configure sampler to be always_off. | ||
| always_off: {} | ||
| # Configure local_parent_sampled sampler. | ||
| local_parent_sampled: | ||
| # Configure sampler to be always_on. | ||
| always_on: {} | ||
| # Configure local_parent_not_sampled sampler. | ||
| local_parent_not_sampled: | ||
| # Configure sampler to be always_off. | ||
| always_off: {} |
There was a problem hiding this comment.
this feels a little overwhelming for newbies, is it needed?
There was a problem hiding this comment.
It does and its also not strictly necessary since without explicitly setting these properties, the parent sampler is instantiated with these default values. This is a policy question (for the opentelemetry-configuration) project:
We have examples which are supposed to represent the default config, complete with env var substitution references to the env var config schema.
If a type (i.e. the parent sampler here) is part of the default config, should we enumerate all of its properties with their default values? Or should we limit the properties to just the required set? Or should we limit the properties to the required set, plus some (subjective) set of properties we deem will be common to configure?
There was a problem hiding this comment.
This comment prompted me to finally get around to fixing the empty objects (i.e. always_off: {} vs always_off:) upstream: open-telemetry/opentelemetry-configuration#134
Demonstrates how declarative config is integrated with the otel java agent and can be used to configure instrumentations.
Related to: open-telemetry/opentelemetry-java-instrumentation#12265