x-pack/metricbeat/module/gcp: Override GCP API endpoint in metric client #40918
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
botelastic
bot
added
the
needs_team
|
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
|
|
mergify
bot
added
the
backport-8.x
pierrehilbert
added
the
Team:Elastic-Agent-Data-Plane
botelastic
bot
removed
the
needs_team
|
Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane) |
pierrehilbert
approved these changes
Sep 20, 2024
pierrehilbert
added
Team:obs-ds-hosted-services
|
Pinging @elastic/obs-ds-hosted-services (Team:obs-ds-hosted-services) |
gpop63
reviewed
Sep 20, 2024
Comment on lines
110
to
115
| Endpoint string `config:"endpoint"` | ||
| opt []option.ClientOption | ||
| period *durationpb.Duration | ||
| organizationID string | ||
| organizationName string | ||
| projectName string |
There was a problem hiding this comment.
Let's keep a new line between these exported and unexported struct fields like it was before.
Suggested change
| Endpoint string `config:"endpoint"` | |
| opt []option.ClientOption | |
| period *durationpb.Duration | |
| organizationID string | |
| organizationName string | |
| projectName string | |
| Endpoint string `config:"endpoint"` | |
| opt []option.ClientOption | |
| period *durationpb.Duration | |
| organizationID string | |
| organizationName string | |
| projectName string |
CHANGELOG.next.asciidoc
Outdated
| @@ -59,6 +59,7 @@ https://github.com/elastic/beats/compare/v8.8.1\...main[Check the HEAD diff] | |||
| - Mark system process metricsets as running if metrics are partially available {pull}40565[40565] | |||
| - Added back `elasticsearch.node.stats.jvm.mem.pools.*` to the `node_stats` metricset {pull}40571[40571] | |||
| - Add GCP organization and project details to ECS cloud fields. {pull}40461[40461] | |||
| - Add `endpoint` field to the GCP configuration in order to append it to the metric client. {issue}40848[40848] {pull}40918[40918] | |||
There was a problem hiding this comment.
Suggested change
| - Add `endpoint` field to the GCP configuration in order to append it to the metric client. {issue}40848[40848] {pull}40918[40918] | |
| - Add support for specifying a custom endpoint for GCP service clients. {issue}40848[40848] {pull}40918[40918] |
Comment on lines
73
to
81
| - module: gcp | ||
| metricsets: | ||
| - compute | ||
| region: "us-" | ||
| project_id: "your project id" | ||
| credentials_file_path: "your JSON credentials file path" | ||
| endpoint: http://your-endpoint | ||
| exclude_labels: false | ||
| period: 1m |
There was a problem hiding this comment.
The yaml block is not properly formatted and it might be redundant to add it again just to showcase the endpoint config param. I would probably add endpoint to an existing config in that file.
Comment on lines
+146
to
+149
| if m.config.Endpoint != "" { | ||
| m.Logger().Warnf("You are using a custom endpoint '%s' for the GCP API calls.", m.config.Endpoint) | ||
| m.config.opt = append(m.config.opt, option.WithEndpoint(m.config.Endpoint)) | ||
| } |
There was a problem hiding this comment.
Add a new line to improve readability.
Suggested change
| if m.config.Endpoint != "" { | |
| m.Logger().Warnf("You are using a custom endpoint '%s' for the GCP API calls.", m.config.Endpoint) | |
| m.config.opt = append(m.config.opt, option.WithEndpoint(m.config.Endpoint)) | |
| } | |
| if m.config.Endpoint != "" { | |
| m.Logger().Warnf("You are using a custom endpoint '%s' for the GCP API calls.", m.config.Endpoint) | |
| m.config.opt = append(m.config.opt, option.WithEndpoint(m.config.Endpoint)) | |
| } |
kaiyan-sheng
approved these changes
Sep 20, 2024
6 tasks
pierrehilbert
pushed a commit
that referenced
this pull request
Sep 23, 2024
… endpoint in metric client (#40938) * x-pack/metricbeat/module/gcp: Override GCP API endpoint in metric client (#40918) * override endpoint to etric client * config and changelog * doc * resolved comments * lint fix (cherry picked from commit 7720c90) * Update CHANGELOG.next.asciidoc --------- Co-authored-by: Linu-Elias <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed commit message
The GCP metric client does not currently allow overriding with a custom endpoint, which is necessary for testing against a mock GCP API endpoint.
Resolved this issue by implementing a check to determine if an endpoint is specified in the GCP configuration, and subsequently overriding the
ClientOptionwith that endpoint.Checklist
CHANGELOG.next.asciidocorCHANGELOG-developer.next.asciidoc.Disruptive User Impact
Author's Checklist
How to test this PR locally
Related issues
Use cases
Screenshots
Logs