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

Generate azure_resource_id #2897

Merged
merged 1 commit into from
Apr 1, 2021
Merged

Generate azure_resource_id #2897

merged 1 commit into from
Apr 1, 2021

Conversation

pmcollins
Copy link
Member

When a resource detection processor is running and we're in an Azure environment, we can use the
Azure resource attributes to generate an azure_resource_id, which is used by the SFx backend.

This functionality aims to provide parity with the SFx Smart Agent's generation of azure_resource_id.

Some code taken from https://github.com/signalfx/signalfx-agent/blob/master/pkg/core/hostid/azure.go#L18

Tested on Azure (standalone VM and Scale Set).

@codecov
Copy link

codecov bot commented Mar 26, 2021

Codecov Report

Merging #2897 (0759c7d) into main (9a5ab2b) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2897      +/-   ##
==========================================
+ Coverage   91.40%   91.42%   +0.01%     
==========================================
  Files         463      463              
  Lines       22834    22874      +40     
==========================================
+ Hits        20872    20912      +40     
  Misses       1468     1468              
  Partials      494      494              
Flag Coverage Δ
integration 69.09% <ø> (-0.07%) ⬇️
unit 90.39% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
internal/splunk/hostid.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9a5ab2b...0759c7d. Read the comment docs.

Code taken from
https://github.com/signalfx/signalfx-agent/blob/master/pkg/core/hostid/azure.go#L18
with minor modifications.

Tested on Azure (standalone VM and Scale Set).
@pmcollins pmcollins marked this pull request as ready for review March 26, 2021 22:18
@pmcollins pmcollins requested a review from a team March 26, 2021 22:18
@jrcamp jrcamp assigned asuresh4 and unassigned kbrockhoff Mar 26, 2021
Copy link
Member

@asuresh4 asuresh4 left a comment

Choose a reason for hiding this comment

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

lgtm, apart from the one comment.

@bogdandrutu bogdandrutu merged commit 7276360 into open-telemetry:main Apr 1, 2021
@pmcollins pmcollins mentioned this pull request Apr 1, 2021
pmatyjasek-sumo pushed a commit to pmatyjasek-sumo/opentelemetry-collector-contrib that referenced this pull request Apr 28, 2021
alexperez52 referenced this pull request in open-o11y/opentelemetry-collector-contrib Aug 18, 2021
…2897)

In Prometheus, `job` and `instance` are the two auto generated labels, however they are both dropped by prometheus receiver. Although these information is still available in `service.name` and `host`:`port`, it breaks the data contract for most Prometheus users (who use `job` and `instance` to consume metrics in their own system).
This PR adds `job` and `instance` as well-known labels in prometheus receiver to fix the issue.

**Link to tracking Issue:** 
open-telemetry/opentelemetry-collector#575
open-telemetry/opentelemetry-collector#2499
open-telemetry/opentelemetry-collector#2363
open-telemetry/wg-prometheus#7
alexperez52 referenced this pull request in open-o11y/opentelemetry-collector-contrib Aug 18, 2021
…orter (#2979)

This is a follow up to #2897.

Fixes #575
Fixes #2499
Fixes #2363
Fixes open-telemetry/wg-prometheus#37
Fixes open-telemetry/wg-prometheus#39
Fixes open-telemetry/wg-prometheus#44

Passing compliance tests:

$ go test --tags=compliance -run "TestRemoteWrite/otelcollector/Job.+" -v ./
=== RUN TestRemoteWrite
=== RUN TestRemoteWrite/otelcollector
=== RUN TestRemoteWrite/otelcollector/JobLabel
=== PAUSE TestRemoteWrite/otelcollector/JobLabel
=== CONT TestRemoteWrite/otelcollector/JobLabel
--- PASS: TestRemoteWrite (10.02s)
--- PASS: TestRemoteWrite/otelcollector (0.00s)
--- PASS: TestRemoteWrite/otelcollector/JobLabel (10.02s)
PASS
ok github.com/prometheus/compliance/remote_write 10.382s
$ go test --tags=compliance -run "TestRemoteWrite/otelcollector/Instance.+" -v ./
=== RUN TestRemoteWrite
=== RUN TestRemoteWrite/otelcollector
=== RUN TestRemoteWrite/otelcollector/InstanceLabel
=== PAUSE TestRemoteWrite/otelcollector/InstanceLabel
=== CONT TestRemoteWrite/otelcollector/InstanceLabel
--- PASS: TestRemoteWrite (10.01s)
--- PASS: TestRemoteWrite/otelcollector (0.00s)
--- PASS: TestRemoteWrite/otelcollector/InstanceLabel (10.01s)
PASS
ok github.com/prometheus/compliance/remote_write 10.291s
$ go test --tags=compliance -run "TestRemoteWrite/otelcollector/RepeatedLabels.+" -v ./
=== RUN TestRemoteWrite
=== RUN TestRemoteWrite/otelcollector
--- PASS: TestRemoteWrite (0.00s)
--- PASS: TestRemoteWrite/otelcollector (0.00s)
testing: warning: no tests to run
PASS
@bd-clara
Copy link

Hi,

This is wrong, azure resource name and hostname can be different

Actually it generates on some of my VM an ID with localhost inside
azure_resource_id:xxxxxxxxxxxxxxxxxxxxxxxxxxxx/xxxxxx-xxx-xxxx-shared-services-support-rg/microsoft.compute/virtualmachines/localhost

Azure Resource ID is part of the VM metadata
curl -s -H Metadata:true --noproxy "*" "https://169.254.169.254/metadata/instance?api-version=2021-02-01"
Two solution, .compute.name or .compute.resourceId

Best regards,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants