Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Update app_signals to application_signals #172

Merged
merged 1 commit into from
May 17, 2024
Merged

Update app_signals to application_signals #172

merged 1 commit into from
May 17, 2024

Conversation

bjrara
Copy link
Contributor

@bjrara bjrara commented May 10, 2024

Issue #, if available:

Description of changes:
Rename app_signals to application_signals for GA release. It's part of the PR: aws/amazon-cloudwatch-agent#1133.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@bjrara bjrara mentioned this pull request May 10, 2024
Closed
okankoAMZ
okankoAMZ approved these changes May 16, 2024
View reviewed changes
Copy link
Contributor

@okankoAMZ okankoAMZ left a comment

Choose a reason for hiding this comment

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

Looks good but I am not sure if need those comments

pkg/instrumentation/defaultinstrumentation_test.go
@@ -39,27 +39,29 @@ func Test_getDefaultInstrumentation(t *testing.T) {
Java: v1alpha1.Java{
Image: defaultJavaInstrumentationImage,
Env: []corev1.EnvVar{
{Name: "OTEL_SMP_ENABLED", Value: "true"}, //TODO: remove in favor of new name once safe
{Name: "OTEL_AWS_APP_SIGNALS_ENABLED", Value: "true"},
{Name: "OTEL_AWS_APP_SIGNALS_ENABLED", Value: "true"}, //TODO: remove in favor of new name once safe
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need this comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, all AWS_APP_SIGNALS will be replaced by AWS_APPLICATION_SIGNALS

pkg/instrumentation/podmutator_test.go
@@ -51,27 +51,29 @@ func TestGetInstrumentationInstanceFromNameSpaceDefault(t *testing.T) {
Java: v1alpha1.Java{
Image: defaultJavaInstrumentationImage,
Env: []corev1.EnvVar{
{Name: "OTEL_SMP_ENABLED", Value: "true"}, //TODO: remove in favor of new name once safe
{Name: "OTEL_AWS_APP_SIGNALS_ENABLED", Value: "true"},
{Name: "OTEL_AWS_APP_SIGNALS_ENABLED", Value: "true"}, //TODO: remove in favor of new name once safe
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, do we need this comment?

pkg/instrumentation/podmutator_test.go
{Name: "OTEL_TRACES_SAMPLER_ARG", Value: "endpoint=http://cloudwatch-agent.amazon-cloudwatch:2000"},
{Name: "OTEL_TRACES_SAMPLER", Value: "xray"},
{Name: "OTEL_EXPORTER_OTLP_PROTOCOL", Value: "http/protobuf"},
{Name: "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", Value: "http://cloudwatch-agent.amazon-cloudwatch:4316/v1/traces"},
{Name: "OTEL_AWS_SMP_EXPORTER_ENDPOINT", Value: "http://cloudwatch-agent.amazon-cloudwatch:4316/v1/metrics"}, //TODO: remove in favor of new name once safe
{Name: "OTEL_AWS_APP_SIGNALS_EXPORTER_ENDPOINT", Value: "http://cloudwatch-agent.amazon-cloudwatch:4316/v1/metrics"},
{Name: "OTEL_AWS_APP_SIGNALS_EXPORTER_ENDPOINT", Value: "http://cloudwatch-agent.amazon-cloudwatch:4316/v1/metrics"}, //TODO: remove in favor of new name once safe
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, do we need this comment?

pkg/instrumentation/defaultinstrumentation_test.go
{Name: "OTEL_TRACES_SAMPLER_ARG", Value: "endpoint=http://cloudwatch-agent.amazon-cloudwatch:2000"},
{Name: "OTEL_TRACES_SAMPLER", Value: "xray"},
{Name: "OTEL_EXPORTER_OTLP_PROTOCOL", Value: "http/protobuf"},
{Name: "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", Value: "https://cloudwatch-agent.amazon-cloudwatch:4316/v1/traces"},
{Name: "OTEL_AWS_SMP_EXPORTER_ENDPOINT", Value: "https://cloudwatch-agent.amazon-cloudwatch:4316/v1/metrics"}, //TODO: remove in favor of new name once safe
{Name: "OTEL_AWS_APP_SIGNALS_EXPORTER_ENDPOINT", Value: "https://cloudwatch-agent.amazon-cloudwatch:4316/v1/metrics"},
{Name: "OTEL_AWS_APP_SIGNALS_EXPORTER_ENDPOINT", Value: "https://cloudwatch-agent.amazon-cloudwatch:4316/v1/metrics"}, //TODO: remove in favor of new name once safe
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, do we need this comment?

pkg/instrumentation/defaultinstrumentation_test.go
@@ -88,27 +90,29 @@ func Test_getDefaultInstrumentation(t *testing.T) {
Java: v1alpha1.Java{
Image: defaultJavaInstrumentationImage,
Env: []corev1.EnvVar{
{Name: "OTEL_SMP_ENABLED", Value: "true"}, //TODO: remove in favor of new name once safe
{Name: "OTEL_AWS_APP_SIGNALS_ENABLED", Value: "true"},
{Name: "OTEL_AWS_APP_SIGNALS_ENABLED", Value: "true"}, //TODO: remove in favor of new name once safe
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, do we need this comment?

pkg/instrumentation/defaultinstrumentation_test.go
{Name: "OTEL_TRACES_SAMPLER_ARG", Value: "endpoint=http://cloudwatch-agent.amazon-cloudwatch:2000"},
{Name: "OTEL_TRACES_SAMPLER", Value: "xray"},
{Name: "OTEL_EXPORTER_OTLP_PROTOCOL", Value: "http/protobuf"},
{Name: "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", Value: "http://cloudwatch-agent.amazon-cloudwatch:4316/v1/traces"},
{Name: "OTEL_AWS_SMP_EXPORTER_ENDPOINT", Value: "http://cloudwatch-agent.amazon-cloudwatch:4316/v1/metrics"}, //TODO: remove in favor of new name once safe
{Name: "OTEL_AWS_APP_SIGNALS_EXPORTER_ENDPOINT", Value: "http://cloudwatch-agent.amazon-cloudwatch:4316/v1/metrics"},
{Name: "OTEL_AWS_APP_SIGNALS_EXPORTER_ENDPOINT", Value: "http://cloudwatch-agent.amazon-cloudwatch:4316/v1/metrics"}, //TODO: remove in favor of new name once safe
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, do we need this comment?

pkg/instrumentation/defaultinstrumentation.go
@@ -63,27 +63,29 @@ func getDefaultInstrumentation(agentConfig *adapters.CwaConfig) (*v1alpha1.Instr
Java: v1alpha1.Java{
Image: javaInstrumentationImage,
Env: []corev1.EnvVar{
{Name: "OTEL_SMP_ENABLED", Value: "true"}, //TODO: remove in favor of new name once safe
{Name: "OTEL_AWS_APP_SIGNALS_ENABLED", Value: "true"},
{Name: "OTEL_AWS_APP_SIGNALS_ENABLED", Value: "true"}, //TODO: remove in favor of new name once safe
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, do we need this comment?

Copy link
Contributor

Choose a reason for hiding this comment

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

Think it exists so that we are backwards compatible until the SDK releases the new version

pkg/instrumentation/defaultinstrumentation.go
{Name: "OTEL_TRACES_SAMPLER_ARG", Value: "endpoint=http://cloudwatch-agent.amazon-cloudwatch:2000"},
{Name: "OTEL_TRACES_SAMPLER", Value: "xray"},
{Name: "OTEL_EXPORTER_OTLP_PROTOCOL", Value: "http/protobuf"},
{Name: "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", Value: exporterPrefix + "cloudwatch-agent.amazon-cloudwatch:4316/v1/traces"},
{Name: "OTEL_AWS_SMP_EXPORTER_ENDPOINT", Value: exporterPrefix + "cloudwatch-agent.amazon-cloudwatch:4316/v1/metrics"}, //TODO: remove in favor of new name once safe
{Name: "OTEL_AWS_APP_SIGNALS_EXPORTER_ENDPOINT", Value: exporterPrefix + "cloudwatch-agent.amazon-cloudwatch:4316/v1/metrics"},
{Name: "OTEL_AWS_APP_SIGNALS_EXPORTER_ENDPOINT", Value: exporterPrefix + "cloudwatch-agent.amazon-cloudwatch:4316/v1/metrics"}, //TODO: remove in favor of new name once safe
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, do we need this comment?

@bjrara
Copy link
Contributor Author

bjrara commented May 16, 2024

All AWS_APP_SIGNALS will be replaced by AWS_APPLICATION_SIGNALS.

@lisguo lisguo changed the title [DO NOT MERGE] Update app_signals to application_signals Update app_signals to application_signals May 17, 2024
@lisguo lisguo merged commit 14bf407 into aws:main May 17, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@okankoAMZ okankoAMZ okankoAMZ approved these changes

@lisguo lisguo lisguo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bjrara @lisguo @okankoAMZ