Backport of Add node id/name to config into release/1.16.x#17796
Merged
chapmanc merged 1 commit intorelease/1.16.xfrom Jun 16, 2023
Merged
Backport of Add node id/name to config into release/1.16.x#17796chapmanc merged 1 commit intorelease/1.16.xfrom
chapmanc merged 1 commit intorelease/1.16.xfrom
Conversation
ad9f664 to
a8785f2
Compare
a888ecd to
fdc169d
Compare
github-team-consul-core-pr-approver
approved these changes
Jun 16, 2023
Collaborator
github-team-consul-core-pr-approver
left a comment
There was a problem hiding this comment.
Auto approved Consul Bot automated PR
fdc169d to
118d371
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport
This PR is auto-generated from #17750 to be assessed for backporting due to the inclusion of the label backport/1.16.
🚨
The person who merged in the original PR is:
@chapmanc
This person should manually cherry-pick the original PR into a new backport PR,
and close this one when the manual backport PR is merged in.
The below text is copied from the body of the original PR.
Description
We currently don't have node name in the NewDeps call. To add it we can keep tacking on new configuration values or we can add new values to the already existing config struct that is built in the builder.
There was a mini refactor required because the cfg is also used as an interface to instantiate the metricsClient. To reduce blast radius of a very large refactor I moved the instantiation of the metricsClient out of the sink initialization and then passed in the metricClient to the sink instead of passing the cfg in as an interface which stops us from using any of the values stored in the config.
I still think we should refactor the function for
NewMetricsClientto not take an interface but for now I think this is an acceptable crawl step. We should work making this code a lot simpler as we are doing quite a bit with the "config" and treating it very much not as just config.Testing & Reproduction steps
E2E tested with real consul-telemetry gateway and E2E test with mock HCP server is showing the labels flow through:
Links
n/a
PR Checklist
Overview of commits