-
Notifications
You must be signed in to change notification settings - Fork 67
feat: [Java] EMF Exporter Implementation #1209
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
base: main
Are you sure you want to change the base?
Conversation
2cf0524 to
746e989
Compare
| Optional<String> awsRegion = getAwsRegionFromConfig(configProps); | ||
|
|
||
| if (awsRegion.isPresent()) { | ||
| if (headers.containsKey(AWS_OTLP_LOGS_GROUP_HEADER) |
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.
Does customer have to set all 3 headers?
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.
It looks like from Python implementation they don't need namespace header:
https://github.com/aws-observability/aws-otel-python-instrumentation/blob/main/aws-opentelemetry-distro/src/amazon/opentelemetry/distro/aws_opentelemetry_configurator.py#L127
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.
Removed validation for namespace header.
…eader, and consolidated unit tests
| * @return Optional containing string with "awsemf" removed if the original OTEL_METRICS_EXPORTER | ||
| * contains "awsemf", otherwise empty Optional if "awsemf" is not found | ||
| */ | ||
| static Optional<String> removeEmfExporterIfEnabled(ConfigProperties configProps) { |
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.
think about simplify the logic, improve readability
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.
added some comments to clarify the method logic
|
|
||
| for (MetricData metric : metricsData) { | ||
| Data<? extends PointData> metricData = metric.getData(); | ||
| if (metricData == null || metricData.getPoints().isEmpty()) { |
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.
can metricData.getPoints() be null and the for loop exits with the NPE exception?
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.
| MetricRecord firstRecord = records.get(0); | ||
| // Get resource from first metric in the collection | ||
| MetricData firstMetric = metricsData.iterator().next(); |
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.
Can you escalate this into the createEmfLog method as part of EMF log creation logic?
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.
Fixed a bug so that the grouping logic to also based on resource attributes and scope
Description of changes:
Java Version of these PRs:
This PR introduces the complete CloudWatch EMF (Embedded Metric Format) exporter implementation for sending OpenTelemetry metrics directly to CloudWatch without requiring a Collector or Agent.
In order to enable this exporter, users MUST set the following environment variables:
OTEL_METRICS_EXPORTER=awsemf-OTEL_EXPORTER_OTLP_LOGS_HEADERS=x-aws-log-group=<log-group-name>,x-aws-log-stream=<log-stream-name>, x-aws-metric-namespace=<namespace>AWS_REGION=<region>ORAWS_DEFAULT_REGION=<region>This PR includes:
TODO:
Testing:
Added unit tests to validate EMF exporter configuration scenarios, including parameterized tests for both valid configurations (supporting AWS_REGION and AWS_DEFAULT_REGION) and invalid configurations (missing headers, wrong exporter type, missing region). The tests ensure the EMF exporter is correctly enabled only when all required environment variables are properly configured.
Manual end to end testing with the following environment variables to ensure the EMF logs show up:
AWS_REGION=us-east-1OTEL_METRICS_EXPORTER=awsemfOTEL_EXPORTER_OTLP_LOGS_HEADERS=x-aws-log-group=test,x-aws-log-stream=default,x-aws-metric-namespace=testNamespaceOTEL_RESOURCE_ATTRIBUTES=service.name=testService,aws.log.group.names=test,cloud.resource_id=agent-12345OTEL_LOGS_EXPORTER=noneOTEL_TRACES_EXPORTER=noneExample EMF log emitted:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.