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

[chore] use type in aerospike receiver #21271

Merged

Conversation

atoulme
Copy link
Contributor

@atoulme atoulme commented May 1, 2023

This uses metadata.Type in the code of the aerospike receiver.

@atoulme atoulme requested a review from a team May 1, 2023 05:50
@atoulme atoulme requested a review from djaglowski as a code owner May 1, 2023 05:50
@github-actions github-actions bot requested a review from antonblock May 1, 2023 05:50
@atoulme atoulme force-pushed the use_type_in_aerospikereceiver branch 2 times, most recently from 8c7f512 to 8414a5b Compare May 1, 2023 08:10
Copy link
Contributor

@evan-bradley evan-bradley left a comment

Choose a reason for hiding this comment

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

I think this will need a changelog entry since we're changing the instrumentation scope name on the emitted metrics. Per the scraping receivers stability guarantees for a component with alpha stability, I think a breaking change will be fine here, but we can lean on the code owners to make that decision.

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Requesting change for the same reason as #21272 (review), @atoulme could you open an issue to track this? It would be good to come to an agreement in the issue and reference it in any changes we make.

@atoulme
Copy link
Contributor Author

atoulme commented May 1, 2023

#21382 is filed

@codeboten codeboten added the on hold This is blocked by another PR/issue label May 3, 2023
@atoulme atoulme force-pushed the use_type_in_aerospikereceiver branch from 8414a5b to b8a9f3b Compare May 11, 2023 03:49
@atoulme atoulme removed the on hold This is blocked by another PR/issue label May 11, 2023
@atoulme atoulme requested a review from codeboten May 11, 2023 03:50
@atoulme
Copy link
Contributor Author

atoulme commented May 11, 2023

No changes to scope name anymore, so we will be good with minimal changes now.

@atoulme
Copy link
Contributor Author

atoulme commented May 12, 2023

@codeboten PTAL

@codeboten codeboten merged commit 45b9b98 into open-telemetry:main May 12, 2023
@github-actions github-actions bot added this to the next release milestone May 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants