-
Notifications
You must be signed in to change notification settings - Fork 497
chore(config): migrate serviceName #4292
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
Conversation
BenchmarksBenchmark execution time: 2025-12-19 16:24:45 Comparing candidate commit 5d9df19 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 3 metrics, 0 unstable metrics. |
|
893a70c to
d859bab
Compare
6c2a483 to
515a4a5
Compare
| cfg.Env = t.config.internalConfig.Env() | ||
| cfg.HTTP = t.config.httpClient | ||
| cfg.ServiceName = t.config.serviceName | ||
| cfg.ServiceName = t.config.internalConfig.ServiceName() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that we have gotten rid of the call to getDDorOtelConfig above, why don't we have to use ConfigProvider here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're using the ConfigProvider here, the internalConfig is built using the ConfigProvider and then assigned as an attribute of t.config the ServiceName in the internalConfig is picked amongst all possible sources.
d859bab to
87da5e8
Compare
515a4a5 to
5d9df19
Compare

What does this PR do?
Migrates tracer to use Config.serviceName instead of its local serviceName.
Note that this PR is narrowly scoped; in the future, we can get rid of setting service name on globalconfig altogether, and likely stop passing in the service name to other products, as these products/packages will just query the Config singleton directly.
Motivation
Go config revamp
Reviewer's Checklist
./scripts/lint.shlocally.Unsure? Have a question? Request a review!