Skip to content

Conversation

Miqueasher
Copy link
Contributor

@Miqueasher Miqueasher commented Oct 16, 2025

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:

  • 2 New imports to views.py (sample app's frontend service for the 'main' instance)
  • Global meter, and histogram/counter added to views.py from OTEL SDK Metrics page
  • Update to environment variables in main.tf
  • Update to jobs & env sections in python-ec2-default-test
  • Update from aws region us-west-2 to us-east-1
  • Update to Cloudwatch agent config to include otlp ports
  • Update to variables.tf to include custom metrics enabled
  • For loop expansion for easier Debugging logs (used in testing & for future)
  • New Validator file to call updated predefined template file
  • Update to PredefinedExpectedTemplate to include new mustache file
  • New mustache file for validation of custom metrics

IMPORTANT
Updates to views.py effect 'python-sample-app.zip which exists in S3 bucket. Proper Zip protocol must be followed for it properly deploy and run. If not it will fail.

Rollback procedure:
A git revert to the most recent SHA (4ca5d60) will be easy to revert back to if necessary.

Passing test runs:
Test 1
Test 2
Test 3
Test 4

References:
https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/AppSignals-CustomMetrics.html#AppSignals-CustomMetrics-OpenTelemetry
https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/CloudWatch-Agent-OpenTelemetry-metrics.html
https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/#exporter-selection
https://opentelemetry.io/docs/languages/sdk-configuration/otlp-exporter/#otel_exporter_otlp_metrics_endpoint
https://opentelemetry.io/docs/specs/otel/metrics/sdk/

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

value: opentelemetry
-
name: telemetry.sdk.version
value: 1.33.1
Copy link
Contributor

Choose a reason for hiding this comment

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

This value can change in the future, don't hardcode it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will be updated in the next push

@majanjua-amzn majanjua-amzn changed the title Custom metrics test [Python] Custom Metrics E2E Test Oct 16, 2025
Copy link
Contributor

@majanjua-amzn majanjua-amzn left a comment

Choose a reason for hiding this comment

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

Seems like the custom_metrics_enabled flag is irrelevant and the feature is independent from the default behaviour. We should just generalize the test and include this validation into the test by default.

-var="cpu_architecture=${{ env.CPU_ARCHITECTURE }}" \
|| deployment_failed=$?
|| deployment_failed=1
Copy link
Contributor

Choose a reason for hiding this comment

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

Undo this

export OTEL_EXPORTER_OTLP_INSECURE=true
export OTEL_SERVICE_NAME=python-sample-application-${var.test_id}
export OTEL_TRACES_SAMPLER=always_on
export OTEL_RESOURCE_ATTRIBUTES="Service=python-sample-application-${var.test_id},Environment=ec2:default"
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this line

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like the changes to this file don't do anything functionally different then before, is that correct?

If so, should probably not change it at all

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will be updated in the next push

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants