Skip to content

Comments

Add potential metric capitalization panic recovery for ApplicationSignals processor#1702

Closed
jefchien wants to merge 2 commits intomainfrom
fix-capitalization
Closed

Add potential metric capitalization panic recovery for ApplicationSignals processor#1702
jefchien wants to merge 2 commits intomainfrom
fix-capitalization

Conversation

@jefchien
Copy link
Contributor

Description of the issue

There have been cases of the agent panicking from the capitalization of metrics in the Application Signals processor.

panic serving [::1]:64029: runtime error: slice bounds out of range [22:21]
goroutine 4539 [running]:
net/http.(*conn).serve.func1()
	net/http/server.go:1903 +0xbe
panic({0x105dc4ca0?, 0xc0011df338?})
	runtime/panic.go:770 +0x132
go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End.deferwrap1()
	go.opentelemetry.io/otel/sdk@v1.27.0/trace/span.go:381 +0x25
go.opentelemetry.io/otel/sdk/trace.(*recordingSpan).End(0xc001358600, {0x0, 0x0, 0xc000ca2000?})
	go.opentelemetry.io/otel/sdk@v1.27.0/trace/span.go:413 +0x9fc
panic({0x105dc4ca0?, 0xc0011df338?})
	runtime/panic.go:770 +0x132
golang.org/x/text/transform.String({0x14f6de010, 0xc0008b23c0}, {0xc0011de240, 0x15})
	golang.org/x/text@v0.16.0/transform/transform.go:650 +0xbb0
golang.org/x/text/cases.Caser.String(...)
	golang.org/x/text@v0.16.0/cases/cases.go:51
github.com/aws/amazon-cloudwatch-agent/plugins/processors/awsapplicationsignals.(*awsapplicationsignalsprocessor).processMetrics(0xc0009a02d0, {0x1061bf3e8, 0xc000e78540}, {0xc000140480?, 0xc00073a844?})
	github.com/aws/amazon-cloudwatch-agent/plugins/processors/awsapplicationsignals/processor.go:140 +0x1a5

It's still unclear why the metric name capitalization is causing a panic.

Description of changes

  • Adds panic recovery, logging, and dropping of the metric if the processor is unable to capitalize the metric.
  • Made the cases.Caser per instance instead of shared within the package to prevent issues with shared state.

https://pkg.go.dev/golang.org/x/text/cases#Caser

// A Caser may be stateful and should therefore not be shared between
// goroutines.

License

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

Tests

Added unit tests.

Requirements

Before commit the code, please do the following steps.

  1. Run make fmt and make fmt-sh
  2. Run make lint

@jefchien jefchien requested a review from a team as a code owner May 23, 2025 20:53
@jefchien jefchien requested a review from bjrara May 23, 2025 21:42
@jefchien jefchien closed this May 27, 2025
thpierce pushed a commit that referenced this pull request May 28, 2025
# Description of the issue
This PR fixes a panic issue described in #1702.

# Description of changes
`golang.org/x/text/cases.Caser.String` is not goroutine safe. In this PR, we initialize a new Caser when a metric processed by Application Signals is found with camel case naming.

# License
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
@jefchien jefchien deleted the fix-capitalization branch December 18, 2025 23:48
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