-
Notifications
You must be signed in to change notification settings - Fork 23
[Python] Custom Metrics E2E Test #471
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?
Changes from 66 commits
f02dcb3
383791c
c74f03f
d89c43b
3e1be9b
f5b6b0e
c5f28fd
3455277
ddd2993
446bd08
75c47a2
0c75867
e285993
dd9e778
a3fedbc
81500c6
72c88c5
b0e450f
0e3aff2
d070aab
67329dc
59b9f12
284c65e
761a7ed
02e58d5
bc4a37a
f3dbdf4
3807ca9
2883c2f
c826fa0
791c366
3c2cbe1
b8cb6c5
9cfa435
f616720
b4356ba
801286c
50d9ebd
a9e2feb
92e8620
effd5d6
ce893f2
c3b25d0
f83f427
79f9a09
29c32d6
7153eaf
1c5f429
710b4c3
0a29d7e
e2e01a8
5d84db6
9a649bc
a4ba51d
dd6c12c
f0f890d
b6bab9e
34087f4
1f74499
db6fda5
ea96f6e
f360d97
523e41b
9149f84
567e84c
fc77459
e3034a5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,7 @@ permissions: | |
contents: read | ||
|
||
env: | ||
E2E_TEST_AWS_REGION: us-west-2 | ||
E2E_TEST_AWS_REGION: us-east-1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will be updated in the next push |
||
CALLER_WORKFLOW_NAME: ${{ inputs.caller-workflow-name }} | ||
SAMPLE_APP_FRONTEND_SERVICE_JAR: s3://aws-appsignals-sample-app-prod-us-west-2-adap/java-main-service-v11.jar | ||
SAMPLE_APP_REMOTE_SERVICE_JAR: s3://aws-appsignals-sample-app-prod-us-west-2-adap/java-remote-service-v11.jar | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,7 +34,7 @@ permissions: | |
contents: read | ||
|
||
env: | ||
E2E_TEST_AWS_REGION: 'us-west-2' # Test uses us-west-2 in the us-east-1 accoun | ||
E2E_TEST_AWS_REGION: 'us-east-1' # Test uses us-east-1 in the us-east-1 account | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will be updated in the next push |
||
CALLER_WORKFLOW_NAME: ${{ inputs.caller-workflow-name }} | ||
METRIC_NAMESPACE: ApplicationSignals | ||
JAVA_VERSION: ${{ inputs.java-version }} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,7 +32,7 @@ permissions: | |
contents: read | ||
|
||
env: | ||
E2E_TEST_AWS_REGION: 'us-west-2' | ||
E2E_TEST_AWS_REGION: 'us-east-1' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will be updated in the next push |
||
CALLER_WORKFLOW_NAME: ${{ inputs.caller-workflow-name }} | ||
NODE_VERSION: ${{ inputs.node-version }} | ||
CPU_ARCHITECTURE: ${{ inputs.cpu-architecture }} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,7 +31,7 @@ permissions: | |
contents: read | ||
|
||
env: | ||
E2E_TEST_AWS_REGION: 'us-west-2' | ||
E2E_TEST_AWS_REGION: 'us-east-1' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will be updated in the next push |
||
CALLER_WORKFLOW_NAME: ${{ inputs.caller-workflow-name }} | ||
PYTHON_VERSION: ${{ inputs.python-version }} | ||
CPU_ARCHITECTURE: ${{ inputs.cpu-architecture }} | ||
|
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -32,6 +32,10 @@ on: | |||||||
required: false | ||||||||
type: string | ||||||||
default: 'github' | ||||||||
custom-metrics-enabled: | ||||||||
required: false | ||||||||
type: boolean | ||||||||
default: false | ||||||||
outputs: | ||||||||
job-started: | ||||||||
value: ${{ jobs.python-ec2-default.outputs.job-started }} | ||||||||
|
@@ -49,6 +53,7 @@ env: | |||||||
CPU_ARCHITECTURE: ${{ inputs.cpu-architecture }} | ||||||||
ADOT_WHEEL_NAME: ${{ inputs.staging-wheel-name }} | ||||||||
OTEL_SOURCE: ${{ inputs.otel-source }} | ||||||||
CUSTOM_METRICS_ENABLED: ${{ inputs.custom-metrics-enabled }} | ||||||||
SAMPLE_APP_ZIP: s3://aws-appsignals-sample-app-prod-${{ inputs.aws-region }}/python-sample-app.zip | ||||||||
E2E_TEST_ACCOUNT_ID: ${{ secrets.APPLICATION_SIGNALS_E2E_TEST_ACCOUNT_ID }} | ||||||||
E2E_TEST_ROLE_NAME: ${{ secrets.APPLICATION_SIGNALS_E2E_TEST_ROLE_NAME }} | ||||||||
|
@@ -171,7 +176,7 @@ jobs: | |||||||
-var="get_adot_wheel_command=${{ env.GET_ADOT_WHEEL_COMMAND }}" \ | ||||||||
-var="language_version=${{ env.PYTHON_VERSION }}" \ | ||||||||
-var="cpu_architecture=${{ env.CPU_ARCHITECTURE }}" \ | ||||||||
|| deployment_failed=$? | ||||||||
-var="custom_metrics_enabled=${{ env.CUSTOM_METRICS_ENABLED }}" \ | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Keep the old line too
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will be updated in the next push |
||||||||
|
||||||||
if [ $deployment_failed -eq 1 ]; then | ||||||||
echo "Terraform deployment was unsuccessful. Will attempt to retry deployment." | ||||||||
|
@@ -252,9 +257,26 @@ jobs: | |||||||
--instance-id ${{ env.MAIN_SERVICE_INSTANCE_ID }} | ||||||||
--rollup' | ||||||||
|
||||||||
- name: Validate CWAgent metrics | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This name doesn't really tell me what we're validating, are we validating custom metrics? If so, we should update the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will be updated in the next push |
||||||||
id: cwagent-metric-validation | ||||||||
if: (success() || steps.log-validation.outcome == 'failure') && !cancelled() | ||||||||
run: ./gradlew validator:run --args='-c python/ec2/default/custom-metric-validation.yml | ||||||||
--testing-id ${{ env.TESTING_ID }} | ||||||||
--endpoint http://${{ env.MAIN_SERVICE_ENDPOINT }} | ||||||||
--remote-service-deployment-name ${{ env.REMOTE_SERVICE_IP }}:8001 | ||||||||
--region ${{ env.E2E_TEST_AWS_REGION }} | ||||||||
--metric-namespace CWAgent | ||||||||
--log-group ${{ env.LOG_GROUP_NAME }} | ||||||||
--service-name python-sample-application-${{ env.TESTING_ID }} | ||||||||
--remote-service-name python-sample-remote-application-${{ env.TESTING_ID }} | ||||||||
--query-string ip=${{ env.REMOTE_SERVICE_IP }} | ||||||||
--instance-ami ${{ env.EC2_INSTANCE_AMI }} | ||||||||
--instance-id ${{ env.MAIN_SERVICE_INSTANCE_ID }} | ||||||||
--rollup' | ||||||||
|
||||||||
- name: Validate generated traces | ||||||||
id: trace-validation | ||||||||
if: (success() || steps.log-validation.outcome == 'failure' || steps.metric-validation.outcome == 'failure') && !cancelled() | ||||||||
if: (success() || steps.log-validation.outcome == 'failure' || steps.metric-validation.outcome == 'failure' || steps.cwagent-metric-validation.outcome == 'failure') && !cancelled() | ||||||||
Comment on lines
-257
to
+279
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only check if that step passed if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will be updated in the next push |
||||||||
run: ./gradlew validator:run --args='-c python/ec2/default/trace-validation.yml | ||||||||
--testing-id ${{ env.TESTING_ID }} | ||||||||
--endpoint http://${{ env.MAIN_SERVICE_ENDPOINT }} | ||||||||
|
@@ -281,7 +303,7 @@ jobs: | |||||||
if: always() | ||||||||
id: validation-result | ||||||||
run: | | ||||||||
if [ "${{ steps.log-validation.outcome }}" = "success" ] && [ "${{ steps.metric-validation.outcome }}" = "success" ] && [ "${{ steps.trace-validation.outcome }}" = "success" ]; then | ||||||||
if [ "${{ steps.log-validation.outcome }}" = "success" ] && [ "${{ steps.cwagent-metric-validation.outcome }}" = "success" ] && [ "${{ steps.metric-validation.outcome }}" = "success" ] && [ "${{ steps.trace-validation.outcome }}" = "success" ]; then | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will be updated in the next push |
||||||||
echo "validation-result=success" >> $GITHUB_OUTPUT | ||||||||
else | ||||||||
echo "validation-result=failure" >> $GITHUB_OUTPUT | ||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,7 @@ permissions: | |
contents: read | ||
|
||
env: | ||
E2E_TEST_AWS_REGION: 'us-west-2' | ||
E2E_TEST_AWS_REGION: 'us-east-1' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will be updated in the next push |
||
E2E_TEST_ACCOUNT_ID: ${{ secrets.APPLICATION_SIGNALS_E2E_TEST_ACCOUNT_ID }} | ||
E2E_TEST_ROLE_NAME: ${{ secrets.APPLICATION_SIGNALS_E2E_TEST_ROLE_NAME }} | ||
ADOT_WHEEL_NAME: ${{ inputs.staging-wheel-name }} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,8 +30,8 @@ variable "architecture" { | |
|
||
variable "region" { | ||
type = string | ||
description = "Lambda function running region, default value is us-west-2" | ||
default = "us-west-2" | ||
description = "Lambda function running region, default value is us-east-1" | ||
default = "us-east-1" | ||
Comment on lines
+33
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, going to stop commenting this on every instance of this issue There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will be updated in the next push |
||
} | ||
|
||
variable "is_canary" { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,16 @@ | ||
{ | ||
"agent": { | ||
"debug": true, | ||
"region": "$REGION" | ||
}, | ||
Comment on lines
-2
to
-5
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why was this removed? Let's keep it in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will be updated in the next push |
||
"traces": { | ||
"traces_collected": { | ||
"application_signals": {} | ||
} | ||
}, | ||
"logs": { | ||
"metrics_collected": { | ||
"application_signals": {} | ||
"application_signals": {}, | ||
"otlp": { | ||
"grpc_endpoint": "0.0.0.0:4317", | ||
"http_endpoint": "0.0.0.0:4318" | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -170,15 +170,24 @@ resource "null_resource" "main_service_setup" { | |
export DJANGO_SETTINGS_MODULE="django_frontend_service.settings" | ||
export OTEL_PYTHON_DISTRO="aws_distro" | ||
export OTEL_PYTHON_CONFIGURATOR="aws_configurator" | ||
export OTEL_METRICS_EXPORTER=none | ||
export OTEL_METRICS_EXPORTER=otlp | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this change anything about the regular test? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but not in a negative way. It is also necessary for the CWAgent to function correctly. |
||
export OTEL_TRACES_EXPORTER=otlp | ||
export OTEL_AWS_APPLICATION_SIGNALS_ENABLED=true | ||
export OTEL_AWS_APPLICATION_SIGNALS_EXPORTER_ENDPOINT=http://localhost:4315 | ||
export OTEL_EXPORTER_OTLP_TRACES_ENDPOINT=http://localhost:4315 | ||
export OTEL_EXPORTER_OTLP_TRACES_PROTOCOL=grpc | ||
export OTEL_EXPORTER_OTLP_METRICS_PROTOCOL=grpc | ||
export OTEL_EXPORTER_OTLP_METRICS_ENDPOINT=localhost:4317 | ||
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" | ||
export AWS_REGION='${var.aws_region}' | ||
export CUSTOM_METRICS_ENABLED='${var.custom_metrics_enabled}' | ||
export RDS_MYSQL_CLUSTER_ENDPOINT='dummy-endpoint' | ||
export RDS_MYSQL_CLUSTER_PASSWORD='ZHVtbXk=' # base64 encoded 'dummy' | ||
export RDS_MYSQL_CLUSTER_USERNAME='dummy-user' | ||
export RDS_MYSQL_CLUSTER_DATABASE='dummy-db' | ||
Comment on lines
+187
to
+190
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't need these, EC2 tests don't test against RDS. If the sample app expects the credentials to be available, make it not be the case There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will be updated in the next push |
||
python${var.language_version} manage.py migrate | ||
nohup opentelemetry-instrument python${var.language_version} manage.py runserver 0.0.0.0:8000 --noreload & | ||
|
||
|
@@ -276,7 +285,7 @@ resource "null_resource" "remote_service_setup" { | |
|
||
# Copy in CW Agent configuration | ||
agent_config='${replace(replace(file("./amazon-cloudwatch-agent.json"), "/\\s+/", ""), "$REGION", var.aws_region)}' | ||
echo $agent_config > amazon-cloudwatch-agent.json | ||
echo "$agent_config" > amazon-cloudwatch-agent.json | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. agent_config goes through the replace function and the quotes are just to prevent word splitting from breaking it. |
||
|
||
# Get and run CW agent rpm | ||
${var.get_cw_agent_rpm_command} | ||
|
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -210,6 +210,9 @@ public enum PredefinedExpectedTemplate implements FileConfig { | |||||||||
PYTHON_EC2_DEFAULT_CLIENT_CALL_METRIC("/expected-data-template/python/ec2/default/client-call-metric.mustache"), | ||||||||||
PYTHON_EC2_DEFAULT_CLIENT_CALL_TRACE("/expected-data-template/python/ec2/default/client-call-trace.mustache"), | ||||||||||
|
||||||||||
/** Python EC2 Default Custom Metrics Test Case Validations */ | ||||||||||
PYTHON_EC2_DEFAULT_AWS_OTEL_CUSTOM_METRIC("/expected-data-template/python/ec2/default/aws-otel-custom-metrics.mustache"), | ||||||||||
Comment on lines
+213
to
+214
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will be updated in the next push |
||||||||||
|
||||||||||
/** Python EC2 Asg Test Case Validations */ | ||||||||||
PYTHON_EC2_ASG_OUTGOING_HTTP_CALL_LOG("/expected-data-template/python/ec2/asg/outgoing-http-call-log.mustache"), | ||||||||||
PYTHON_EC2_ASG_OUTGOING_HTTP_CALL_METRIC("/expected-data-template/python/ec2/asg/outgoing-http-call-metric.mustache"), | ||||||||||
|
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 change stuff for unrelated 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.
Will be updated in the next push