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

feat(jaeger-v2): Create default config for all-in-one #4842

Merged
merged 5 commits into from
Oct 15, 2023

Conversation

yurishkuro
Copy link
Member

@yurishkuro yurishkuro commented Oct 15, 2023

Which problem is this PR solving?

In Jaeger V1 the all-in-one binary can be run without any arguments / configuration, but since jaeger-v2 is built on OTel Collector it requires a config file. This is unnecessary friction for local / development use of all-in-one.

Description of the changes

  • Allows all-in-one with memory storage to be run without any additional config file
  • Hardcodes a configuration in the code, relying on default configuration from each component.

How was this change tested?

  • Ran locally
$ go run -tags=ui ./cmd/jaeger-v2

Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Yuri Shkuro <github@ysh.us>
@codecov
Copy link

codecov bot commented Oct 15, 2023

Codecov Report

All modified lines are covered by tests ✅

see 1 file with indirect coverage changes

📢 Thoughts on this report? Let us know!.

@yurishkuro yurishkuro marked this pull request as ready for review October 15, 2023 04:23
@yurishkuro yurishkuro requested a review from a team as a code owner October 15, 2023 04:23
Signed-off-by: Yuri Shkuro <github@ysh.us>
@yurishkuro yurishkuro merged commit e0da3c9 into jaegertracing:main Oct 15, 2023
31 checks passed
@yurishkuro yurishkuro deleted the v2-all-in-one branch October 15, 2023 15:31
@yurishkuro yurishkuro added the changelog:exprimental Change to an experimental part of the code label Oct 17, 2023
yurishkuro added a commit that referenced this pull request Oct 21, 2023
## Which problem is this PR solving?
- Part of #4843 
- Improvement on #4842, where all-in-one configuration was created via
createDefaultConfig methods of extensions called by OTel automatically.
This resulted in the memstore always being present in the config and
always instantiated, which is not the desired behavior.

## Description of the changes
- The cmd.RunE interceptor is changed to provide an internal value to
the `--config` flag if no flag was present on cmd line
- This avoids creating the collector manually, once we set the flag we
always delegate back to official OTel code
- The value for the config is embedded in the binary and passed using
`yaml:...`, which is one of the official config providers in OTel

## How was this change tested?
- Ran all-in-one manually

Signed-off-by: Yuri Shkuro <github@ysh.us>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:exprimental Change to an experimental part of the code
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

1 participant