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

[receiver/hostmetrics] Unify feature gates for direction removal #12105

Merged
merged 1 commit into from
Jul 6, 2022

Conversation

dmitryax
Copy link
Member

@dmitryax dmitryax commented Jul 6, 2022

This change replaces set of independent feature gates for transition from the metrics with direction attribute with two unified feature gates that are applied on all the metrics reported by the hostmetrics receiver.

  • receiver.hostmetricsreceiver.emitMetricsWithDirectionAttribute
  • receiver.hostmetricsreceiver.emitMetricsWithoutDirectionAttribute

Also this introduces the following changes to the transition process:

  1. I shifts the transition schedule one release forward since disc scraper is not yet ready to emit the new metrics
  2. It removes the warnings. They will be introduced in 0.56.0 when all the scrapers are aligned with the schedule.
  3. It introduces an option to emit both the deprecated and the new metrics. This can be useful for users during the transition period.

Link to tracking Issue: #11815

@dmitryax dmitryax requested review from a team and mx-psi July 6, 2022 06:54
@dmitryax dmitryax force-pushed the host-metrics-feature-gates branch 2 times, most recently from b7c7278 to e0838ca Compare July 6, 2022 07:10
@dmitryax dmitryax added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Jul 6, 2022
@dmitryax
Copy link
Member Author

dmitryax commented Jul 6, 2022

Added "Skip Changelog" to pass CI until #12106 is merged

@dmitryax
Copy link
Member Author

dmitryax commented Jul 6, 2022

@codeboten PTAL and let me know what do you think. I would like to include it in the 0.55.0 release, feel free to remove the blocker label if you disagree

@dmitryax dmitryax added the release:blocker The issue must be resolved before cutting the next release label Jul 6, 2022
@dmitryax dmitryax force-pushed the host-metrics-feature-gates branch 4 times, most recently from d74f351 to a1f436e Compare July 6, 2022 08:00
@tigrannajaryan
Copy link
Member

@open-telemetry/collector-contrib-approvers please review/merge or remove the release:blocker label.

Copy link
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

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

This looks good to me, but would prefer if @codeboten had a look too if he has more context

@tigrannajaryan tigrannajaryan requested a review from codeboten July 6, 2022 15:06
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.

Just one question and a comment, this consolidation makes sense to me. However it might be worth capturing here that it wont apply to the disk scraper just yet as that change hasn't been implemented.

receiver/hostmetricsreceiver/README.md Outdated Show resolved Hide resolved
@dmitryax dmitryax force-pushed the host-metrics-feature-gates branch from a1f436e to 25a1d53 Compare July 6, 2022 16:24
This change replaces set of independent feature gates for transition from the metrics with `direction` attribute with two unified feature gates that are applied on all the metrics reported by the hostmetrics receiver.
- `receiver.hostmetricsreceiver.emitMetricsWithDirectionAttribute`
- `receiver.hostmetricsreceiver.emitMetricsWithoutDirectionAttribute`

Also this introduces the following changes to the transition process:
1. I shifts the transition schedule one release forward since disc scraper is not yet ready to emit the new metrics
2. It removes the warnings. They will be introduced in 0.56.0 when all the scrapers are aligned with the schedule.
3. It introduces an option to emit both the deprecated and the new metrics. This can be useful for users during the transition.
@dmitryax dmitryax force-pushed the host-metrics-feature-gates branch from 25a1d53 to 8144153 Compare July 6, 2022 16:28
@dmitryax dmitryax removed the Skip Changelog PRs that do not require a CHANGELOG.md entry label Jul 6, 2022
@dmitryax
Copy link
Member Author

dmitryax commented Jul 6, 2022

Test failed: https://github.com/open-telemetry/opentelemetry-collector-contrib/runs/7218945997?check_suite_focus=true

This seems to be a flaky unrelated prometheus receiver test, submitted #12126

@tigrannajaryan
Copy link
Member

Please merge once the build passes. I will wait for it and will start the contrib release procedure.

@dmitryax dmitryax merged commit 195abd8 into open-telemetry:main Jul 6, 2022
@dmitryax
Copy link
Member Author

dmitryax commented Jul 6, 2022

@tigrannajaryan, merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:blocker The issue must be resolved before cutting the next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants