-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
x-pack/metricbeat/module/gcp: Add Organization ID and display name to cloud labels #40461
Changes from 11 commits
9d45226
9f2752d
7a56bd9
7228ac4
be57da1
67e8aa6
8ff45bc
38096b5
6b55a2b
de598db
909f6f4
7c0e761
31d8ec6
0bff706
33dc055
06ca307
ae616bc
bef89ea
5bc8500
4522f71
bc83ba0
e75868d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,16 +27,20 @@ func NewStackdriverCollectorInputData(ts *monitoringpb.TimeSeries, projectID, zo | |
|
||
// NewStackdriverMetadataServiceForTimeSeries apart from having a long name takes a time series object to return the | ||
// Stackdriver canonical Metadata extractor | ||
func NewStackdriverMetadataServiceForTimeSeries(ts *monitoringpb.TimeSeries) MetadataService { | ||
func NewStackdriverMetadataServiceForTimeSeries(ts *monitoringpb.TimeSeries, organizationID, organizationName string) MetadataService { | ||
return &StackdriverTimeSeriesMetadataCollector{ | ||
timeSeries: ts, | ||
timeSeries: ts, | ||
organizationID: organizationID, | ||
organizationName: organizationName, | ||
} | ||
} | ||
|
||
// StackdriverTimeSeriesMetadataCollector is the implementation of MetadataCollector to collect metrics from Stackdriver | ||
// common TimeSeries objects | ||
type StackdriverTimeSeriesMetadataCollector struct { | ||
timeSeries *monitoringpb.TimeSeries | ||
timeSeries *monitoringpb.TimeSeries | ||
organizationID string | ||
organizationName string | ||
} | ||
|
||
// Metadata parses a Timeseries object to return its metadata divided into "unknown" (first object) and ECS (second | ||
|
@@ -53,14 +57,19 @@ func (s *StackdriverTimeSeriesMetadataCollector) Metadata(ctx context.Context, i | |
|
||
ecs := mapstr.M{ | ||
ECSCloud: mapstr.M{ | ||
ECSCloudAccount: mapstr.M{ | ||
ECSCloudProject: mapstr.M{ | ||
ECSCloudAccountID: accountID, | ||
ECSCloudAccountName: accountID, | ||
gpop63 marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we need to change the names of these constants; even if the values are correct ("id" and "name"), the name is misleading. They contain "CloudAccount" while we use them for the ECSCloudProject: ECSCloudAccountID = "id"
ECSCloudAccountName = "name" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
}, | ||
ECSCloudProvider: "gcp", | ||
}, | ||
} | ||
|
||
if s.organizationID != "" { | ||
_, _ = ecs.Put(ECSCloud+"."+ECSCloudAccount+"."+ECSCloudAccountID, s.organizationID) | ||
} | ||
if s.organizationName != "" { | ||
_, _ = ecs.Put(ECSCloud+"."+ECSCloudAccount+"."+ECSCloudAccountName, s.organizationName) | ||
} | ||
if availabilityZone != "" { | ||
_, _ = ecs.Put(ECSCloud+"."+ECSCloudAvailabilityZone, availabilityZone) | ||
|
||
|
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.
Should this be a bug fix or enhancement?
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.
Yeah, it's a challenging call. It feels both.
IMO, this change is 75% an enhancement and 25% a fix. It's not a blocker; I'll leave the final call to you, @Linu-Elias.