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

[Metricbeat] Add PubSub metricset to Google Cloud Platform module #15536

Merged
merged 17 commits into from
Mar 4, 2020

Conversation

sayden
Copy link
Contributor

@sayden sayden commented Jan 14, 2020

Adds PubSub metricset to Google Cloud Platform module.

Requires a slight change in code to adapt the Stackdriver filter which doesn't allow to specify a region when requesting metrics from PubSub service.

Everything else is docs and yaml

cf. #15812

@sayden sayden requested a review from a team as a code owner January 14, 2020 12:40
@sayden sayden self-assigned this Jan 14, 2020
"version": "8.0.0"
},
"googlecloud": {
"labels": {
Copy link
Contributor

Choose a reason for hiding this comment

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

just curious is the label key here resource.subscription_id? I understand the value is test-subscription, but not sure whats the resource part. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is more like a property but we thought it was a bit too "complex" to talk about metadata, labels, resources, properties... so we joined all of them into "labels".

This information comes on each response of the Stackdriver API. The one you mention here is a resource label as you can see in the link (follow MonitoredResource link too). You can also find "system" labels, "user" labels and instead of having a bunch of user.labels.*, resource.labels.*, system.labels.* we swapped the .labels part to just have everything under the key labels.*

For example, resource labels in compute are the instance type or the storage type. User labels are a key-value you can set in gcp console.

In this case, PubSub don't have the specific implementation to gather user labels so we are missing the data that the user might input in the console. This data uses PubSub API and not Stackdriver API, that's why each metricset requires an specific implementation inside the stackdriver metricset

I'm not sure if I did a very good job explaining this now, frankly 😅

req := &monitoringpb.ListTimeSeriesRequest{
Name: "projects/" + r.config.ProjectID,
Interval: r.interval,
View: monitoringpb.ListTimeSeriesRequest_FULL,
Filter: r.getFilterForMetric(m),
Filter: filter,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: is this change necessary? 😬

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I might mess it up solving the conflicts. Glad that you spot it to take a closer look 😬

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it was only that, I reverted it 😬

Copy link
Contributor

@kaiyan-sheng kaiyan-sheng left a comment

Choose a reason for hiding this comment

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

LGTM!

@sayden sayden merged commit 246225b into elastic:master Mar 4, 2020
kaiyan-sheng added a commit that referenced this pull request Mar 26, 2020
…5536) (#17264)

(cherry picked from commit 246225b)

Co-authored-by: Mario Castro <mariocaster@gmail.com>
kaiyan-sheng added a commit that referenced this pull request Mar 26, 2020
…5536) (#17263)

(cherry picked from commit 246225b)

Co-authored-by: Mario Castro <mariocaster@gmail.com>
@sayden sayden added the test-plan Add this PR to be manual test plan label Mar 31, 2020
@andresrc andresrc added the test-plan-added This PR has been added to the test plan label Apr 1, 2020
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
…astic#15536) (elastic#17264)

(cherry picked from commit 71e8ca1)

Co-authored-by: Mario Castro <mariocaster@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Metricbeat Metricbeat Team:Integrations Label for the Integrations team test-plan Add this PR to be manual test plan test-plan-added This PR has been added to the test plan v7.7.0 v7.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants