Skip to content

Switch version from function to string in azurevm#8346

Closed
sonalgaud12 wants to merge 3 commits into
open-telemetry:mainfrom
sonalgaud12:azurevm
Closed

Switch version from function to string in azurevm#8346
sonalgaud12 wants to merge 3 commits into
open-telemetry:mainfrom
sonalgaud12:azurevm

Conversation

@sonalgaud12
Copy link
Copy Markdown
Contributor

Change Version() func to const Version string in azurevm
Part of #8272

@sonalgaud12 sonalgaud12 requested a review from a team as a code owner January 4, 2026 09:46
@github-actions github-actions Bot requested a review from pyohannes January 4, 2026 09:46
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.3%. Comparing base (ac3d638) to head (e8c0699).
⚠️ Report is 13 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #8346   +/-   ##
=====================================
  Coverage   82.3%   82.3%           
=====================================
  Files        193     193           
  Lines      13769   13769           
=====================================
  Hits       11334   11334           
  Misses      2030    2030           
  Partials     405     405           
Files with missing lines Coverage Δ
detectors/azure/azurevm/vm.go 91.3% <100.0%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.


if metadata.VMId != nil {
attributes = append(attributes, semconv.HostID(*metadata.VMId))
attributes = append(attributes, semconv.HostIDKey.String(*metadata.VMId))
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.

Can you explain why you want to adjust it this way?

For now, the functional style seems to be somewhat better.

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.

This follows issue #8272 to switch from helper functions to key-based strings for semantic conventions. I see your point about the functional style being cleaner. I just implemented as per my understanding of the issue, so in case we prefer the current implementation, I am happy to close the PR

Copy link
Copy Markdown
Member

@flc1125 flc1125 Jan 4, 2026

Choose a reason for hiding this comment

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

I think you might have misunderstood. You can refer to #8142 to help you understand the purpose and requirements of #8272.

@flc1125 flc1125 added the invalid This doesn't seem right label Jan 5, 2026
Copy link
Copy Markdown
Member

@dmathieu dmathieu left a comment

Choose a reason for hiding this comment

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

As @flc1125 mentions, this PR does the wrong change.

@sonalgaud12 sonalgaud12 closed this Jan 7, 2026
@sonalgaud12 sonalgaud12 deleted the azurevm branch March 12, 2026 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

invalid This doesn't seem right

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants