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

added all-in-one deployment and configmap for jaeger-v2 #606

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

hellspawn679
Copy link

@hellspawn679 hellspawn679 commented Oct 1, 2024

What this PR does

Which issue this PR fixes

is a part of jaegertracing/jaeger#5767

  • fixes #

Checklist

  • DCO signed
  • Commits are GPG signed
  • Chart Version bumped
  • Title of the PR starts with chart name ([jaeger] or [jaeger-operator])
  • README.md has been updated to match version/contain new values

Comment on lines 11 to 13
- opentracing
- tracing
- instrumentation
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- opentracing
- tracing
- instrumentation
- opentelemetry
- tracing

helm install jaeger-v2 ./ -f my-config-values.yaml
```

This command installs the Jaeger chart and uses `my-config-values.yaml` for overriding any configuration specified in the default `values.yaml`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does "overriding" mean? Does it completely ignore the default file, or performs some sort of merge?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's say you have specified

jaeger_storage:
   backends:
     some_store:
       memory:
         max_traces: 90000
     another_store:
       memory:
         max_traces: 90000

but didn't specified

remote_sampling:
   adaptive:
     sampling_store: some_store
     initial_sampling_probability: 0.1

so it will use default value of 0.1 from vaules.yaml

Comment on lines 32 to 34
adaptive:
sampling_store: some_store
initial_sampling_probability: 0.1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we want adaptive sampling to be the default.

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

Successfully merging this pull request may close these issues.

2 participants