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 Google Cloud Platform module #14829

Merged
merged 59 commits into from
Jan 8, 2020

Conversation

sayden
Copy link
Contributor

@sayden sayden commented Nov 27, 2019

ONGOING work on docs bust most code is ready to go.

Seed PR for the Google Cloud Platform module for Metricbeat.

It includes the following:

  • Stackdriver metricset
  • Compute metricset based on Stackdriver as config based module

Ignore the following Metricsets which are already included in the PR for testing purposes but they are not going to be merged yet (they'll be removed before merging):

  • Storage
  • Firebase
  • Firestore
  • Loadbalancing
  • PubSub

Some vocabulary for people new to Google Cloud

You can find some translations for GCP services in AWS:

  • Stackdriver -> Cloudwatch
  • Compute -> EC2
  • PubSub -> SQS
  • Storage (GCS) -> S3
  • Firebase / Firestore -> ~DynamoDB
  • Bigquery -> ~Redshift+Athena

Labels / Metadata

You'll see lots of mentions to Metadata inside the code. This refers to two different entities within GCP: labels and metadata. For Elasticsearch purposes both can be considered metadata so whenever you read "label" or "metadata" it's going to be treated as the same thing at the end of the pipeline.

Grouping of events

The way that GCP labels metrics is somehow complex to generate "service based events". They export their metrics individually so you don't request "compute metrics" or "metrics of this compute instance" but instead you have to request "give all cpu_utilization values of compute instances" so a single response will bring one or more values per instance for a specified timeframe for all your instances. That's a single response.

For example, a request for CPU utilization can return (in pseudocode):

{
	"metadata": {
		"zone": "eu-central-1",
		"project": "project1"
    },
    "metric": "cpu_utilization",
	"points": [
		{
			"time": 1,
			"value": 2,
			"metadata": {
				"instance": "instance-1"
			}
		},
		{
			"time": 2,
			"value": 2,
			"metadata": {
				"instance": "instance-1"
			}
		}
	]
}

Then, a new call must be done to (in this example it will be Compute API) to request Instance metadata (like working group, network group, user labels or user metadata which is associated only to the instance and not to a particular metrics like CPU). Then you get data like this (again, in pseudocode)

{
    "instance":"instance-1",
    "metadata":{
        "user":{
            "key":"value"
        },
        "system":{
            "key":"value"
        }
    },
    ...
}

At the end, both response for that particular metric must be grouped into a single event that share some common metadata. For compute this includes instance_id and availability zone apart from timestamp. Each service requires an specifici implementation to get non-stackdriver metadata. The service metadata implementation is only developed for Compute at the moment and can be seen in googlecloud/stackdriver/compute, the rest of the services uses only metadata provided by Stackdriver.

ECS

Metadata returned from Stackdriver is ECS compliant for Compute metadata (mainly availability zone, account id and cloud provider, instance id and instance name). Some of the metadata might be written out of the ECS fields. More deployment configurations plus testing is needed find them all.

Modules

All services from https://cloud.google.com/monitoring/api/metrics_gcp can be added as more configuration. Tests until now shows no problem but their specific metadata must be developed separatedly for each of them.

Limitations

You cannot set period under 300s (you can right now, but it won't return any metric). I think it's some kind of limitation of Stackdriver because their metrics are sampled each 60 to 300 seconds.

Happy reviewing :)

Sorry for the big PR, it was impossible to make it smaller

@sayden sayden self-assigned this Nov 27, 2019
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
x-pack/metricbeat/module/googlecloud/constants.go Outdated Show resolved Hide resolved
@sayden sayden marked this pull request as ready for review November 29, 2019 12:09
@sayden sayden requested a review from a team as a code owner November 29, 2019 12:09
Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

First pass through the code, I haven't found anything serious but I think that some things would need to be polished. Thanks!

vendor/vendor.json Outdated Show resolved Hide resolved
vendor/vendor.json Outdated Show resolved Hide resolved
SERVICE_COMPUTE = "compute"
SERVICE_PUBSUB = "pubsub"
SERVICE_FIRESTORE = "firestore"
SERVICE_STORAGE = "storage"
Copy link
Member

Choose a reason for hiding this comment

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

+1, please use camel case for these constants

x-pack/metricbeat/module/googlecloud/metadata.go Outdated Show resolved Hide resolved
- firebase
- storage
- loadbalancing
zone: "your zone"
Copy link
Contributor

Choose a reason for hiding this comment

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

can we specify more than one zone here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. We can't specify it right now. The idea was to maintain first version as simple as possible. It's actually possible to request all metrics for a project without zone filter or even request various zones but we are moving slow yet and see how it goes because the code to request metrics and convert them using lightweight modules is pretty complex already.

Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps it would be interesting to put a real zone here so things won't fail if they start the module out of the box?

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 prefer that things do fail explicitly so that a user with machines in Europe that runs Metricbeat will have an specific error saying zone "your zone" not found instead of silent errors of simply not sending any event because there are no machines in that zone/region which may lead to think that Metricbeat is not working properly (it's your fault because you didn't set the correct zone, but that's implicit)

"id": "elastic-metricbeat"
},
"provider": "googlecloud",
"instance": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a separate API to get more info for each compute instance? For example the machine type, status and etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yes! And I'm actually using it already but I completely forgot to attach machine type too! Thanks for the heads up!

@sayden sayden merged commit 8be7745 into elastic:master Jan 8, 2020
@jsoriano jsoriano added needs_backport PR is waiting to be backported to other branches. test-plan Add this PR to be manual test plan v7.6.0 labels Jan 9, 2020
@jsoriano
Copy link
Member

jsoriano commented Jan 9, 2020

@sayden this will need to be backported to 7.x.

@sayden sayden removed the needs_backport PR is waiting to be backported to other branches. label Jan 15, 2020
sayden added a commit to sayden/beats that referenced this pull request Jan 15, 2020
Includes Stackdriver and Compute Metricset
# Conflicts:
#	NOTICE.txt
#	vendor/vendor.json
@exekias
Copy link
Contributor

exekias commented Jan 15, 2020

I just saw this doesn't have a changelog, could you add it in a different PR?

@exekias exekias added the needs_backport PR is waiting to be backported to other branches. label Jan 15, 2020
@exekias
Copy link
Contributor

exekias commented Jan 15, 2020

as 7.6 branch was already created, could you also add another backport to that one?

sayden added a commit to sayden/beats that referenced this pull request Jan 15, 2020
Includes Stackdriver and Compute Metricset

(cherry picked from commit 8be7745)

# Conflicts:
#	NOTICE.txt
#	vendor/vendor.json
@sayden sayden removed the needs_backport PR is waiting to be backported to other branches. label Jan 15, 2020
exekias pushed a commit that referenced this pull request Jan 15, 2020
…ule (#15575)

* [Metricbeat] Add Google Cloud Platform module (#14829)

Includes Stackdriver and Compute Metricset

(cherry picked from commit 8be7745)
@kaiyan-sheng kaiyan-sheng self-assigned this Jan 15, 2020
@kaiyan-sheng
Copy link
Contributor

@sayden I just started testing this PR with compute metricset. Curious, why we have metrics from the same instance but in different events? I also see that you have metrics separated into cpu, disk, firewall and etc in https://github.com/elastic/beats/tree/master/x-pack/metricbeat/module/googlecloud/compute/_meta. Why they are not in the same event/metric from the same instance?

@kaiyan-sheng
Copy link
Contributor

Bug found during testing: #15613

@kaiyan-sheng kaiyan-sheng added the test-plan-regression Manually testing this PR found a regression label Jan 16, 2020
@kaiyan-sheng
Copy link
Contributor

Missing exported field in documentation for compute metricset: #15776

@kaiyan-sheng
Copy link
Contributor

kaiyan-sheng commented Jan 23, 2020

enhancement request(no need for 7.6) for adding regions as a config parameter: #15780

@kaiyan-sheng
Copy link
Contributor

potential sensitive data in labels.metadata: #15782

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Metricbeat Metricbeat needs testing notes Team:Integrations Label for the Integrations team test-plan Add this PR to be manual test plan test-plan-regression Manually testing this PR found a regression v7.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants