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] [GCP] Fix compute metadata #36338
Conversation
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
@kaiyan-sheng I unskipped the test in a draft PR and the test fails cause it runs on 8.8.2. Do you think it is a good idea to backport this so that we can validate it now instead of waiting for the next release? |
@tdancheva good point! I would consider this as a bug fix and let's backport it. |
* [Metricbeat] [GCP] Fix compute metadata (#36338) * don't remake map * only add instances from project_id * fix ecs cloud region and AZ bug * Revert "fix ecs cloud region and AZ bug" This reverts commit 349bf6b. * Revert "only add instances from project_id" This reverts commit 89f6ab5. * add changelog entry (cherry picked from commit 63a147a) * Update CHANGELOG.next.asciidoc --------- Co-authored-by: Gabriel Pop <94497545+gpop63@users.noreply.github.com> Co-authored-by: kaiyan-sheng <kaiyan.sheng@elastic.co>
Proposed commit message
A few things I noticed while debugging:
NewMetadataService
is invoked based on theperiod
set in the config e.g.1m
, therefore thecomputeInstances
map will clear by itself, no need to remake it again ininstance
. If the period is1m
, are there any risks of having stale metadata?beats/x-pack/metricbeat/module/gcp/metrics/compute/metadata.go
Lines 25 to 32 in 138e623
ID
is also callingMetadata
, and intimeSeriesGrouped
both are calledbeats/x-pack/metricbeat/module/gcp/metrics/compute/identity.go
Lines 16 to 17 in 138e623
beats/x-pack/metricbeat/module/gcp/metrics/timeseries.go
Lines 33 to 43 in 138e623
ListTimeSeries
call inMetric
gets instances from other projects, not only from theproject_id
specified in the config. TheAggregatedList
call will not be able to find those instances.beats/x-pack/metricbeat/module/gcp/metrics/metrics_requester.go
Lines 39 to 53 in 138e623
I think the problem is that there are many
Metadata
calls and clearing thecomputeInstances
map would trigger the computeAggregatedList
call a lot of times, and this would go on forever since that API call can't get metadata for all instances - there are also Kubernetes pods/nodes (we can't get metadata for them) that send compute metrics, not only VM instances.By not remaking the
computeInstances
map, we only make 1AggregatedList
call per period specified in config (1m).Checklist
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.Author's Checklist
How to test this PR locally
Related issues
Use cases
Screenshots
Logs