-
Notifications
You must be signed in to change notification settings - Fork 435
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
[awsfirehose] Add routing rules for metrics ingested from Firehose #9916
Conversation
🚀 Benchmarks reportTo see the full report comment with |
packages/awsfirehose/data_stream/logs/_dev/test/pipeline/test-apigateway-log.json-expected.json
Show resolved
Hide resolved
packages/awsfirehose/data_stream/logs/_dev/test/pipeline/test-apigateway-log.json-expected.json
Show resolved
Hide resolved
Do you want to consider adding the details of permissions needed, mentioned here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the documentation, i could find the below text
(Optional) To change the output format from the default format for your scenario, choose Change output format. The supported formats are JSON, OpenTelemetry 1.0.0, and OpenTelemetry 0.7.0.
Among these supported formats, which one the user must select?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Non-blocking suggestion to add subobjects: false
to dimensions.
@agithomas Thanks for the review!
Good point! I will add this in https://github.com/elastic/observability-docs/blob/main/docs/en/observability/cloud-monitoring/aws/monitor-aws-firehose.asciidoc!
User must choose JSON or otel 1.0.0! Good point, I need to add it in https://github.com/elastic/observability-docs/blob/main/docs/en/observability/cloud-monitoring/aws/monitor-aws-firehose.asciidoc as well! Thanks!! Hmm after reviewing the integration, I think these info and more doc needs to be added for this package too. I will update both doc! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
I would only double-check if we can completely leverage ecs@mappings
for all remaining fields and drop the ecs.yml
files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
💚 Build Succeeded
History
|
Quality Gate failedFailed conditions |
Package awsfirehose - 1.1.0 containing this change is available at https://epr.elastic.co/search?package=awsfirehose |
Proposed commit message
This PR is to add
routing_rules.yml
for metrics ingested from Firehose. Without this integration or user specific set the parameter in Firehose, metrics coming into Elasticsearch by default goes tometrics-aws.cloudwatch-default
. Idea to reroute is by checking the value ofaws.cloudwatch.namespace
field in each document to decide which data stream to route the document to. For example ifaws.cloudwatch.namespace
equals toAWS/EC2
, then this document should go tometrics-aws.ec2_metrics-default
.For s3 daily storage and s3 request, we check for the actual metric name. If metric name is either
BucketSizeBytes
orNumberOfObjects
, this document goes toaws.s3_daily_storage
. If it's other metric name but still withAWS/S3
namespace, documents go toaws.s3_request
.Checklist
changelog.yml
file.Related issues