-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merge Jaeger OTEL configuration with OTEL config file #2198
Comments
sgtm |
In general sounds good, although
and
Might need more detail on how components would be merged/overridden. So we should explore some different scenarios that may occur. |
I will include one example in this post. If that is not enough could you please provide a specific configuration example? The configuration provided in the OTEL conf file would have higher precedence over the "hardcoded" default configuration. The precedence is from lower to higher:
The following example results in this configuration:
SPAN_STORAGE_TYPE=elasticsearch go run main.go --es.server-urls=http://localhost:9200/ --es.num-shars=3 Conf file: exporters:
jaeger_elasticsearch:
es:
server-urls: http://elastic:9200
processors:
batch:
disabled: true
attributes:
actions:
- key: db.table
action: delete |
@pavolloffay Thanks for the example. I wasn't sure from the original description whether the config for the individual components would be merged or just replaced - and from your example above it appears (e.g. for This is fine when additional or existing (overridding) parameters are provided. But if the existence of a property in the default config needed to be removed, this would not be possible. Given the limited parameters that would be defined in the default config - with others being explicitly provided via CLI/env or in the OTC config, this is probably not going to be a problem, but worth bearing in mind. The other approach, which would avoid this, would be to simply replace the config for any component defined in the OTC config - but then we would loose the benefit of shared config (e.g. ES parameters) supplied via CLI. So just to clarify the problem, if a property is defined in a default config, it is not possible to remove it - only overwrite it with a different value. But as said, not likely to be a problem with our simple default configs. So I think your approach is good. |
@pavolloffay Reviewing the sampling strategies PR reminded me I wanted to ask about how the pipelines will be merged? For example, if users want to add new processors, extensions or exporters to the existing pipeline? What if they want to also create a second pipeline, with some processors being defined in either or both - possibly with different configs (so different names)? |
At the moment Jaeger OTEL collector can be started without configuration fine. When the configuration file is missing Jaeger uses an opinionated configuration and installs a set of OTEL components (Jaeger receiver, batch processor, etc.). These components can be configured via old Jaeger flags.
When the file is provided the default configuration is not being used and the configuration in flags are not used at all - unless the component (ES exporter) if enabled in the config.
My proposal is to make Jaeger OTEL collector use its opinionated configuration even when the configuration file is provided. The configurations would be merged and the file would have higher precedence (it allows to disable components if needed). Now the configuration from flags would be always used. In the most common cases, users would just provide an additional configuration in the conf file - e.g. attribute processor or tune retry/batch processors.
This would also simplify deployment in of Jaeger OTEL collector in the Jaeger Operator. The operator could expose CR field (yaml) where the additional configuration could be specified.
The text was updated successfully, but these errors were encountered: