Add GCP Metrics Integration #2849

kgeckhart · 2023-01-27T18:35:51Z

PR Description

Introduces a GCP metrics integration based on https://github.com/prometheus-community/stackdriver_exporter

Which issue(s) this PR fixes

Notes to the Reviewer

PR Checklist

CHANGELOG updated
Documentation added
Tests updated

CLAassistant · 2023-01-27T18:35:57Z

All committers have signed the CLA.

karengermond

I left some comments. I included some links to the Writers' Toolkit. Here's a link to the main page: https://grafana.com/docs/writers-toolkit/.

karengermond · 2023-01-30T14:14:57Z

docs/sources/configuration/integrations/gcp-exporter-config.md

+
+As an example,
+for this load balancing metric
+![gcp-exporter-config-metric](gcp-exporter-config-metric.png)


This image isn't showing up in my preview. Is it really needed? Could you link to the documentation topic rather than taking a screenshot of it? If you decide to keep the screenshot, here are guidelines: https://grafana.com/docs/writers-toolkit/writing-guide/image-guidelines/. Here's the section on where to store assets: https://grafana.com/docs/writers-toolkit/writing-guide/image-guidelines/#where-to-store-media-assets. You're supposed to use the asset uploader application and not store images locally.

That's odd, what are you using to preview? https://github.com/grafana/agent/blob/2756dd7e62c13cb34f7a7dc70067dfcfda0c6d83/docs/sources/configuration/integrations/gcp-exporter-config.md renders it without issue.

GCP's docs aren't not really that descriptive about the three pieces of data below so I figured a screenshot breaking down what each piece means would be helpful. I can upload it if needed.

I uploaded it let me know if it's helpful or should be removed

docs/sources/configuration/integrations/gcp-exporter-config.md

karengermond · 2023-01-30T14:39:09Z

docs/sources/configuration/integrations/gcp-exporter-config.md

+After deciding how the agent will obtain credentials you will need to ensure the account is set up with the IAM role `roles/monitoring.viewer`.
+Since the exporter gathers all of its data from GCP Monitoring APIs this is the only permission needed.
+
+## Configuration Examples


I think you should include the reference section above the examples.

docs/sources/configuration/integrations/gcp-exporter-config.md

kgeckhart · 2023-01-31T16:35:25Z

@karengermond, I fixed up all of your doc suggestions. Thank you for the feedback, would you be able to take another pass?

@marctc would it be possible to get a technical review sometime this week? Thanks.

marctc · 2023-01-31T16:37:59Z

@marctc would it be possible to get a technical review sometime this week? Thanks.

yes sir, tomorrow will be ready 👍

marctc · 2023-02-01T10:20:38Z

pkg/integrations/gcp_exporter/gcp_exporter.go

+	"golang.org/x/oauth2/google"
+	"google.golang.org/api/monitoring/v3"
+	"google.golang.org/api/option"
+	"gopkg.in/yaml.v3"


I think we prefer to use v2, for some reason v3 is giving undesired results when marshaling sometimes

marctc · 2023-02-01T10:27:26Z

pkg/integrations/gcp_exporter/gcp_exporter.go

+}
+
+func (c *Config) Validate() error {
+	var configErrors []string


I don't have strong opinion, but maybe is better use multierror here?

marctc · 2023-02-01T10:34:18Z

pkg/integrations/gcp_exporter/gcp_exporter.go

+	}
+
+	googleClient.Timeout = httpTimeout
+	googleClient.Transport = rehttp.NewTransport(


I didn't look deeper, but you reckon we could use this instead? It's already in the dependencies and seems to be more popular than rehttp.

I gave it a shot like,

googleClient, err := google.DefaultClient(ctx, monitoring.MonitoringReadScope) if err != nil { return nil, fmt.Errorf("error creating Google client: %v", err) } retryClient := retryablehttp.NewClient() retryClient.RetryMax = 4 retryClient.RetryWaitMin = time.Second retryClient.RetryWaitMax = 5 * time.Second retryClient.Logger = nil googleClient.Transport = &retryablehttp.RoundTripper{Client: retryClient} monitoringService, err := monitoring.NewService(ctx, option.WithHTTPClient(googleClient), )

Unfortunately, the end result seemed to disable the auth from the google.DefaultClient 😢

marctc · 2023-02-02T10:27:45Z

docs/sources/configuration/integrations/gcp-exporter-config.md

+![gcp-exporter-config-metric-example](https://grafana.com/media/docs/agent/gcp-exporter-config-metric-example.png)
+
+The following list shows its attributes: \
+monitored_resource = `https_lb_rule`\


Why are these \ used for? github rendering is not very happy about it

That's weird github is rendering it for me without issue,

The \'s create a newline without a paragraph break. I do need to remove the \ from line 24 though

marctc · 2023-02-02T10:30:18Z

pkg/integrations/gcp_exporter/gcp_exporter.go

+		rehttp.RetryAll(
+			rehttp.RetryMaxRetries(4),
+			rehttp.RetryStatuses(http.StatusServiceUnavailable)),
+		rehttp.ExpJitterDelay(time.Second, 5*time.Second),


cc @spartan0x117 if you need inspiration about the jitter

marctc

Left one comment but overall LGTM!

kgeckhart · 2023-02-02T12:10:12Z

Thanks @marctc! I'll fix that up and rebase again after #2802 is merged.

karengermond

LGTM. Some minor edits to the code blocks in the gcp-exporter-config.md.

docs/sources/configuration/integrations/gcp-exporter-config.md

Co-authored-by: Karen Germond <110922559+karengermond@users.noreply.github.com>

kgeckhart requested review from thepalbi and ferruvich January 27, 2023 18:35

kgeckhart requested review from karengermond, rgeyer, gaantunes, captncraig and marctc as code owners January 27, 2023 18:35

karengermond reviewed Jan 30, 2023

View reviewed changes

kgeckhart force-pushed the daniele/hackathon-2022-12-keep-it-saasy-gcp-metrics branch 2 times, most recently from 3ef3907 to 32a2a9c Compare January 31, 2023 16:33

marctc reviewed Feb 1, 2023

View reviewed changes

marctc reviewed Feb 2, 2023

View reviewed changes

marctc approved these changes Feb 2, 2023

View reviewed changes

kgeckhart force-pushed the daniele/hackathon-2022-12-keep-it-saasy-gcp-metrics branch from a6c7358 to 1b8fcf2 Compare February 2, 2023 13:07

marctc requested a review from karengermond February 2, 2023 13:55

karengermond approved these changes Feb 2, 2023

View reviewed changes

ferruvich and others added 9 commits February 2, 2023 16:34

wip: gcp metrics exporter - first version

53e7a16

adding integration to agent

1e68088

removing cancelable context with short timeout

ed1b4ab

updating comment

fd56dc7

adding configuration

0e47cc6

removing api key

241b581

commenting args

3fcb93c

this integration should be multiplex in v2

0c03927

Rename to gcp_exporter and add config validation

01537e4

kgeckhart and others added 13 commits February 2, 2023 16:34

Hash config to use as instance key to make it v2 compatible

667b614

Add tests

6e6a990

Support multiple project_ids and fix bug with filterPrefix validation

c066ee4

Add docs

0cd7835

Update changelog

e72b05d

Upgrade to latest stackdriver release

66f52c4

Fix docs error

3a4496b

Refactor documentation based on code review feedback

0d20116

Upload example metric screenshot

2122979

Code review feedback for yaml and multierror

660fdd3

Fix unecessary trailing backslash

dccd372

Apply suggestions from code review

2ae4dc5

Co-authored-by: Karen Germond <110922559+karengermond@users.noreply.github.com>

fix rebase on go.mod

75d7f69

kgeckhart force-pushed the daniele/hackathon-2022-12-keep-it-saasy-gcp-metrics branch from 69875a4 to 75d7f69 Compare February 2, 2023 21:37

marctc merged commit fbfc748 into main Feb 3, 2023

marctc deleted the daniele/hackathon-2022-12-keep-it-saasy-gcp-metrics branch February 3, 2023 09:26

github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Mar 10, 2024

github-actions bot locked as resolved and limited conversation to collaborators Mar 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add GCP Metrics Integration #2849

Add GCP Metrics Integration #2849

kgeckhart commented Jan 27, 2023

CLAassistant commented Jan 27, 2023 •

edited

Loading

karengermond left a comment

karengermond Jan 30, 2023

kgeckhart Jan 30, 2023

kgeckhart Jan 31, 2023

karengermond Jan 30, 2023

kgeckhart commented Jan 31, 2023

marctc commented Jan 31, 2023

marctc Feb 1, 2023

marctc Feb 1, 2023

marctc Feb 1, 2023

kgeckhart Feb 1, 2023

marctc Feb 2, 2023

kgeckhart Feb 2, 2023 •

edited

Loading

marctc Feb 2, 2023

marctc left a comment

kgeckhart commented Feb 2, 2023

karengermond left a comment

Add GCP Metrics Integration #2849

Add GCP Metrics Integration #2849

Conversation

kgeckhart commented Jan 27, 2023

PR Description

Which issue(s) this PR fixes

Notes to the Reviewer

PR Checklist

CLAassistant commented Jan 27, 2023 • edited Loading

karengermond left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kgeckhart commented Jan 31, 2023

marctc commented Jan 31, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kgeckhart Feb 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marctc left a comment

Choose a reason for hiding this comment

kgeckhart commented Feb 2, 2023

karengermond left a comment

Choose a reason for hiding this comment

CLAassistant commented Jan 27, 2023 •

edited

Loading

kgeckhart Feb 2, 2023 •

edited

Loading