-
Notifications
You must be signed in to change notification settings - Fork 581
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
Add support for detecting MIG attributes on GCE VMs #6538
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6538 +/- ##
=====================================
Coverage 68.5% 68.5%
=====================================
Files 200 200
Lines 16768 16782 +14
=====================================
+ Hits 11490 11501 +11
- Misses 4933 4936 +3
Partials 345 345
|
Please address the lint failures. |
// TODO: semconv.GCPGceInstanceGroupManagerNameKey | ||
gcpGceInstanceGroupManagerNameKey = attribute.Key("gcp.gce.instance_group_manager.name") | ||
// TODO: semconv.GCPGceInstanceGroupManagerZoneKey | ||
gcpGceInstanceGroupManagerZoneKey = attribute.Key("gcp.gce.instance_group_manager.zone") | ||
// TODO: semconv.GCPGceInstanceGroupManagerRegionKey | ||
gcpGceInstanceGroupManagerRegionKey = attribute.Key("gcp.gce.instance_group_manager.region") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please open an issue to track the work needed to resolve these TODOs and reference the issue in this comment.
r.attrs = append(r.attrs, gcpGceInstanceGroupManagerRegionKey.String(mig.Location)) | ||
} | ||
} else { | ||
r.errs = append(r.errs, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing test for this error.
if mig, err := detect(); err == nil { | ||
if mig.Name != "" { | ||
r.attrs = append(r.attrs, gcpGceInstanceGroupManagerNameKey.String(mig.Name)) | ||
} | ||
switch mig.Type { | ||
case gcp.Zone: | ||
r.attrs = append(r.attrs, gcpGceInstanceGroupManagerZoneKey.String(mig.Location)) | ||
case gcp.Region: | ||
r.attrs = append(r.attrs, gcpGceInstanceGroupManagerRegionKey.String(mig.Location)) | ||
} | ||
} else { | ||
r.errs = append(r.errs, err) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation can be reduced by restructuring:
if mig, err := detect(); err == nil { | |
if mig.Name != "" { | |
r.attrs = append(r.attrs, gcpGceInstanceGroupManagerNameKey.String(mig.Name)) | |
} | |
switch mig.Type { | |
case gcp.Zone: | |
r.attrs = append(r.attrs, gcpGceInstanceGroupManagerZoneKey.String(mig.Location)) | |
case gcp.Region: | |
r.attrs = append(r.attrs, gcpGceInstanceGroupManagerRegionKey.String(mig.Location)) | |
} | |
} else { | |
r.errs = append(r.errs, err) | |
} | |
mig, err := detect() | |
if err != nil { | |
r.errs = append(r.errs, err) | |
return | |
} | |
if mig.Name != "" { | |
r.attrs = append(r.attrs, gcpGceInstanceGroupManagerNameKey.String(mig.Name)) | |
} | |
switch mig.Type { | |
case gcp.Zone: | |
r.attrs = append(r.attrs, gcpGceInstanceGroupManagerZoneKey.String(mig.Location)) | |
case gcp.Region: | |
r.attrs = append(r.attrs, gcpGceInstanceGroupManagerRegionKey.String(mig.Location)) | |
} |
This adds support for two new resource attributes,
gcp.gce.instance_group_manager.name
andgcp.gce.instance_group_manager.{zone,region}
, that indicate the managed instance group that a Compute Engine VM is part of. The equivalent Collector change is in open-telemetry/opentelemetry-collector-contrib#36142