Skip to content
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

Closed
pavolloffay opened this issue Apr 24, 2020 · 5 comments · Fixed by #2211
Closed

Merge Jaeger OTEL configuration with OTEL config file #2198

pavolloffay opened this issue Apr 24, 2020 · 5 comments · Fixed by #2211

Comments

@pavolloffay
Copy link
Member

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.

@ghost ghost added the needs-triage label Apr 24, 2020
@yurishkuro
Copy link
Member

sgtm

@objectiser
Copy link
Contributor

In general sounds good, although

The configurations would be merged and the file would have higher precedence (it allows to disable components if needed)

and

or tune retry/batch processors.

Might need more detail on how components would be merged/overridden. So we should explore some different scenarios that may occur.

@pavolloffay
Copy link
Member Author

pavolloffay commented Apr 27, 2020

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:

  1. Legacy Jaeger configuration: hardcoded default values < jaeger conf file < env vars < flags (applies viper precedence)
  2. OTEL conf file

The following example results in this configuration:

  • ES exporter will use URL http://elastic:9200/ and 3 shards (default is 5)
  • Batch processor will be disabled (by default it is enabled)
  • Attributes processor will be used (it's not used by default).
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

@objectiser
Copy link
Contributor

objectiser commented Apr 27, 2020

@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 jaeger_elasticsearch) it is merged.

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.

@objectiser
Copy link
Contributor

@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)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants