Skip to content

[chore][receiver/vcenter] Migrate feature gate to mdatagen#47558

Merged
mx-psi merged 10 commits into
open-telemetry:mainfrom
ogulcanaydogan:refactor/46116-migrate-featuregate-vcenterreceiver
Apr 29, 2026
Merged

[chore][receiver/vcenter] Migrate feature gate to mdatagen#47558
mx-psi merged 10 commits into
open-telemetry:mainfrom
ogulcanaydogan:refactor/46116-migrate-featuregate-vcenterreceiver

Conversation

@ogulcanaydogan

Copy link
Copy Markdown
Contributor

Description

Migrates the receiver.vcenter.resourcePoolMemoryUsageAttribute feature gate from a manual featuregate.GlobalRegistry().MustRegister() call in metrics.go to the feature_gates: section in metadata.yaml, allowing mdatagen to generate the gate registration code automatically.

This is part of the ongoing effort tracked in #46116.

Changes

  • Added feature_gates: section to metadata.yaml
  • Generated internal/metadata/generated_feature_gates.go via go generate
  • Replaced manual MustRegister call in metrics.go with reference to generated gate variable
  • Updated scraper_test.go to use the generated gate variable
  • Removed forbidigo linter exclusion for receiver/vcenterreceiver/ from .golangci.yml

Link to tracking issue

Fixes part of #46116

Testing

  • go build ./... passes
  • go test ./... passes

Assisted-by: Claude Sonnet 4.6

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 824d759367

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread receiver/vcenterreceiver/metadata.yaml Outdated
Comment on lines +22 to +23
from_version: "v0.104.0"
reference_url: https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/33607

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve feature gate retirement version

The migration drops the previous WithRegisterToVersion("v0.107.0") setting for receiver.vcenter.resourcePoolMemoryUsageAttribute, so the generated gate now has no retirement target and is documented as To Version: N/A. This changes behavior from the prior registration in metrics.go and removes the intended deprecation/removal signal for this gate, which can leave a long-expired compatibility flag effectively permanent.

Useful? React with 👍 / 👎.

@ogulcanaydogan

Copy link
Copy Markdown
Contributor Author

The to_version field cannot be used with stage: alpha in mdatagen — the generator validates this and returns an error: "to_version is not supported for the alpha stage". The original manual registration combining alpha stage with WithRegisterToVersion was already inconsistent. The migration preserves the gate's alpha stage without a retirement version, which is the correct representation.

Comment on lines 296 to 298

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't understand this change

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That deep-copy hunk was a leftover from an earlier branch, the same change was already merged into main via #47512. It no longer appears in this PR's diff after the rebase

@ogulcanaydogan ogulcanaydogan force-pushed the refactor/46116-migrate-featuregate-vcenterreceiver branch from 824d759 to c3511b7 Compare April 13, 2026 12:00

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

no need to have a changelog note, add [chore] to the title instead

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done, removed the changelog entry and updated the title to [chore][receiver/vcenter]

@ogulcanaydogan ogulcanaydogan changed the title [receiver/vcenter] Migrate feature gate to mdatagen [chore][receiver/vcenter] Migrate feature gate to mdatagen Apr 13, 2026
"receiver.vcenter.resourcePoolMemoryUsageAttribute",
featuregate.StageAlpha,
featuregate.WithRegisterDescription("Enables the memory usage type attribute for the vcenter.resource_pool.memory.usage metric"),
featuregate.WithRegisterReferenceURL("https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/33607"),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't understand how this issue is related to this feature gate, could you point me to a comment/diff that proves the relationship?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

PR #33741 lists #33607 as the tracking issue that's the PR where the memory_usage_type attribute was added to vcenter.resource_pool.memory.usage

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Understood, let's list https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/33741 as the reference URL instead, even if it is not an issue I think it would be clearer

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done, updated to #33741

@ogulcanaydogan ogulcanaydogan force-pushed the refactor/46116-migrate-featuregate-vcenterreceiver branch from 1dbc56e to cee3c7b Compare April 13, 2026 14:15
@ogulcanaydogan ogulcanaydogan force-pushed the refactor/46116-migrate-featuregate-vcenterreceiver branch from a82b3a9 to 4a9d3ef Compare April 15, 2026 12:44

@mx-psi mx-psi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Changes LGTM, one question about the linter change

Comment thread .golangci.yml Outdated
Comment on lines +182 to +184
- linters:
- forbidigo
path: receiver/vcenterreceiver/
path: receiver/prometheusreceiver/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't this be removed instead?

@atoulme atoulme added waiting-for-code-owners and removed processor/metricsgeneration Metrics Generation processor labels Apr 23, 2026
@mx-psi

mx-psi commented Apr 24, 2026

Copy link
Copy Markdown
Member

Looks like there are some merge conflicts

Move metricsgeneration.MatchAttributes feature gate registration from
manual featuregate.GlobalRegistry().MustRegister() in processor.go to
the metadata.yaml file, letting mdatagen generate the registration code.

Remove the forbidigo lint exclusion for this package from .golangci.yml
since it no longer uses manual feature gate registration.

Fixes open-telemetry#46116
Add .chloggen entry for PR open-telemetry#47512 and fix import ordering
in generated_package_test.go per make gogci.
Move receiver.vcenter.resourcePoolMemoryUsageAttribute feature gate
registration from manual MustRegister call to metadata.yaml feature_gates
section, allowing mdatagen to generate the gate registration code.

Part of open-telemetry#46116.

Assisted-by: Claude Sonnet 4.6
Reviewer requested no changelog entry for this chore-level change.

Assisted-by: Claude Sonnet 4.6
Signed-off-by: Ogulcan Aydogan <ogulcanaydogan@hotmail.com>
…-telemetry#33741

Signed-off-by: Ogulcan Aydogan <ogulcanaydogan@hotmail.com>
…-telemetry#33741

Signed-off-by: Ogulcan Aydogan <ogulcanaydogan@hotmail.com>
Signed-off-by: Ogulcan Aydogan <ogulcanaydogan@hotmail.com>
Signed-off-by: Ogulcan Aydogan <ogulcanaydogan@hotmail.com>
@ogulcanaydogan ogulcanaydogan force-pushed the refactor/46116-migrate-featuregate-vcenterreceiver branch from 0eca254 to c75d0d6 Compare April 24, 2026 14:45
Comment thread .golangci.yml Outdated
- linters:
- forbidigo
path: receiver/vcenterreceiver/
path: receiver/jaegerreceiver/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why change it instead of removing?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch, fixed. The block is now removed entirely.

The vcenterreceiver forbidigo exclusion is no longer needed after
migrating feature gates to mdatagen-generated config. Remove the
exclusion block entirely rather than substituting another receiver.

Signed-off-by: Ogulcan Aydogan <ogulcanaydogan@hotmail.com>
@mx-psi mx-psi merged commit 3abb763 into open-telemetry:main Apr 29, 2026
192 checks passed
@otelbot

otelbot Bot commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Thank you for your contribution @ogulcanaydogan! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey. If you are getting started contributing, you can also join the CNCF Slack channel #opentelemetry-new-contributors to ask for guidance and get help.

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