-
Notifications
You must be signed in to change notification settings - Fork 23
[Java] Custom Metrics E2E Test #479
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
thpierce
left a comment
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.
Why is there no change to the following files?
.github/workflows/java-ec2-default-test.ymlterraform/python/ec2/default/README.md
| name: cloud.platform | ||
| value: aws_ec2 | ||
| value: aws_ec2 |
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.
Duplicated.
| name: cloud.platform | |
| value: aws_ec2 | |
| value: aws_ec2 | |
| name: cloud.platform | |
| value: aws_ec2 |
| # Get and run the sample application with configuration | ||
| aws s3 cp ${var.sample_app_jar} ./main-service.jar | ||
| aws s3 cp ${var.sample_app_jar} ./main-service-delete-me.jar |
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.
Undo
| OTEL_RESOURCE_ATTRIBUTES="service.name=$${SERVICE_NAME},deployment.environment.name=$${DEPLOYMENT_ENVIRONMENT_NAME},Internal_Org=Financial,Business Unit=Payments,Region=us-east-1,aws.application_signals.metric_resource_keys=Business Unit&Region&Organization" \ | ||
| OTEL_INSTRUMENTATION_COMMON_EXPERIMENTAL_CONTROLLER_TELEMETRY_ENABLED=true \ | ||
| nohup java -XX:+UseG1GC -jar main-service.jar &> nohup.out & | ||
| nohup java -XX:+UseG1GC -jar main-service-delete-me.jar &> nohup.out & |
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.
Undo
| # Get and run the sample application with configuration | ||
| aws s3 cp ${var.sample_remote_app_jar} ./remote-service.jar | ||
| aws s3 cp ${var.sample_remote_app_jar} ./remote-service-delete-me.jar |
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.
Undo
| OTEL_RESOURCE_ATTRIBUTES=service.name=sample-remote-application-${var.test_id} \ | ||
| OTEL_INSTRUMENTATION_COMMON_EXPERIMENTAL_CONTROLLER_TELEMETRY_ENABLED=true \ | ||
| nohup java -XX:+UseG1GC -jar remote-service.jar &> nohup.out & | ||
| nohup java -XX:+UseG1GC -jar remote-service-delete-me.jar &> nohup.out & |
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.
Undo
| @ResponseBody | ||
| public String awssdkCall(@RequestParam(name = "testingId", required = false) String testingId) { | ||
|
|
||
| logger.info("Incrementing custom counter - OpenTelemetry available: {}", GlobalOpenTelemetry.get() != null); |
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.
Not needed
| logger.info("Incrementing custom counter - OpenTelemetry available: {}", GlobalOpenTelemetry.get() != null); |
| private final LongCounter pipelineCounter; | ||
| private final DoubleHistogram pipelineHistogram; | ||
| private final LongUpDownCounter pipelineGauge; |
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.
| private final LongCounter pipelineCounter; | |
| private final DoubleHistogram pipelineHistogram; | |
| private final LongUpDownCounter pipelineGauge; | |
| private final LongCounter customPipelineCounter; | |
| private final DoubleHistogram customPipelineHistogram; | |
| private final LongUpDownCounter customPipelineGauge; |
| .build(); | ||
|
|
||
| SdkMeterProvider pipelineMeterProvider = SdkMeterProvider.builder() | ||
| .setResource(Resource.getDefault()) |
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.
Don't you need to create the resource with servicename/deploymentenvname?
| .build(); | ||
|
|
||
| MetricReader pipelineMetricReader = PeriodicMetricReader.builder(pipelineMetricExporter) | ||
| .setInterval(Duration.ofSeconds(60)) |
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.
| .setInterval(Duration.ofSeconds(60)) | |
| .setInterval(Duration.ofSeconds(1)) |
| private static final LongUpDownCounter gauge = meter.upDownCounterBuilder("agent_based_gauge").build(); | ||
|
|
||
| // Pipeline-based metrics (initialized in constructor) | ||
| private final Meter pipelineMeter; |
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.
Not needed
| private final Meter pipelineMeter; |
This PR adds custom metrics to the existing sample app to verify public documentation is correct and functionality continues to work as changes happen in the future (Regression testing). Specifically for OpenTelemetry metrics using the CloudWatch Agent.
The changes made in this PR are:
Git revert back to last passing CSHA: 82e7075
<Can we safely revert this commit if needed? If not, detail what must be done to safely revert and why it is needed.>
Ensure you've run the following tests on your changes and include the link below:
To do so, create a
test.ymlfile withname: Testand workflow description to test your changes, then remove the file for your PR. Link your test run in your PR description. This process is a short term solution while we work on creating a staging environment for testing.NOTE: TESTS RUNNING ON A SINGLE EKS CLUSTER CANNOT BE RUN IN PARALLEL. See the needs keyword to run tests in succession.
e2e-playgroundin us-east-1 and eu-central-2e2e-playgroundin us-east-1 and eu-central-2e2e-playgroundin us-east-1 and eu-central-2By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.