Adding Statuscode Handler to Agent Health Extension #1423

Paramadon · 2024-11-12T20:47:17Z

Description of the Issue

The CloudWatch Agent lacked an agenthealth configuration to monitor API health by tracking HTTP status codes for specific responses. Monitoring the status codes (200, 400, 408, 419, and 429) across all APIs is critical for diagnosing issues and ensuring comprehensive observability of API behaviors. Without this configuration, users were unable to quickly identify trends in API health metrics or correlate specific status codes with performance issues.

Changes Made

Added agenthealth Configuration:
- Configured the CloudWatch Agent to track the following status codes across all APIs:
  - 200: Success
  - 400: Bad Request
  - 408: Request Timeout
  - 419: Authentication Timeout
  - 429: Too Many Requests
- This ensures that health metrics for these common response codes are captured and reported.
Updated CloudWatch Agent Configuration JSON:
- Introduced a new section under the metrics collection configuration to specify agenthealth metrics.
- Ensured compatibility with existing metrics and logs collection by integrating the new configuration seamlessly.
Validated the Configuration:
- Tested the updated configuration with the CloudWatch Agent to ensure the new metrics are collected and reported correctly.
- Confirmed that the metrics appear as expected in CloudWatch for all specified APIs.

Impact

Provides real-time visibility into API status codes, helping identify anomalies or patterns that indicate issues.
Enhances observability for critical services by adding detailed health metrics.

Testing

Deployed the updated CloudWatch Agent health configuraion
Verified that metrics for status codes 200, 400, 408, 419, and 429 are captured in the header as you can see from the image below:

…s from status codes

…andler

mitali-salvi

Could you fix the linting issues ?

mitali-salvi · 2024-11-21T16:59:59Z

extension/agenthealth/config.go

@@ -12,6 +12,7 @@ import (
 type Config struct {
 	IsUsageDataEnabled bool              `mapstructure:"is_usage_data_enabled"`
 	Stats              agent.StatsConfig `mapstructure:"stats"`
+	StatusCodeOnly     *bool             `mapstructure:"is_status_code_only,omitempty"`


nitpick - can be renamed to is_only_status_code_enabled
also keep it consistent with IsUsageDataEnabled with IsOnlyStatusCodeEnabled

Also any reason to make it a pointer ?

A bool with omitempty in the tag is enough.

Agreed with mitali. Instead of a pointer, can we treat the default value as false?

Additionally, can you explain a bit more what this field is used for? It seems like a way to set the agent health extension such that it turns the other things off. I think that may be confusing because you could set IsUsageDataEnabled to true and also set StatusCodeOnly to true, so it's not clear what it would actually do (without looking at the implementation).

That makes sense. I will rename it. The pointer is used so we can emit it even if it's empty (nil). As for its functionality, I can add a comment, but the main purpose is to ensure that we only use the statuscodehandler when calling the agenthealthextension for a processor, exporter, or receiver, without including other handlers like processstats, clientstats, etc.

extension/agenthealth/handler/stats/agent/agent.go

extension/agenthealth/handler/stats/provider/process_test.go

extension/agenthealth/handler/stats/provider/interval.go

tool/clean/clean_host/clean_host.go

extension/agenthealth/handler/stats/agent/agent_test.go

extension/agenthealth/handler/stats/agent/agent.go

extension/agenthealth/handler/stats/agent/agent_test.go

extension/agenthealth/handler/stats/handler.go

extension/agenthealth/handler/stats/provider/statuscode.go

internal/ecsservicediscovery/containerinstanceprocessor.go

extension/agenthealth/handler/stats/provider/statuscode.go

jefchien · 2024-11-21T20:59:01Z

extension/agenthealth/config.go

@@ -12,6 +12,7 @@ import (
 type Config struct {
 	IsUsageDataEnabled bool              `mapstructure:"is_usage_data_enabled"`
 	Stats              agent.StatsConfig `mapstructure:"stats"`
+	StatusCodeOnly     *bool             `mapstructure:"is_status_code_only,omitempty"`


A bool with omitempty in the tag is enough.

cmd/amazon-cloudwatch-agent/amazon-cloudwatch-agent.go

extension/agenthealth/handler/stats/provider/interval.go

extension/agenthealth/handler/stats/agent/agent.go

jefchien · 2024-11-21T21:23:48Z

extension/agenthealth/config.go

@@ -12,6 +12,7 @@ import (
 type Config struct {


IMO status code stats should have to be enabled. agenthealth/metrics and agenthealth/traces won't have any allowlisted operations, but are still going to have the handlers created and attached.

I agree with this and I changed it so that we would have to enable statuscodes

extension/agenthealth/handler/stats/agent/agent.go

extension/agenthealth/handler/stats/provider/statuscode.go

dricross · 2024-11-22T21:44:53Z

extension/agenthealth/handler/stats/provider/statuscode.go

+	value, loaded := h.statsByOperation.LoadOrStore(operation, &[5]int{})
+	if !loaded {
+		log.Printf("Initializing stats for operation: %s", operation)
+	}
+	stats := value.(*[5]int)
+
+	h.updateStatusCodeCount(stats, statusCode, operation)
+
+	h.statsByOperation.Store(operation, stats)


Hmm, these lines consist of a read-modify-write operation. Two threads running this at the same time could clobber each others updates. Only the latter thread will have it's updates applied. As an example:

Thread A and B are both handling an HTTP response for the same operation. HTTP response is 200 for both threads.

Thread A loads current values, let's say stats[0] = 10

Thread B loads current values, also sees stats[0] = 10

Both threads now have a copy stats array

B adds 1 to its copy, stats[0] = 11

B stores its result

A adds 1 to it's copy, stats[0] = 11

A stores its result

We end up with stats[0]=11 for the operation instead of stats[0]=12. Perhaps its not actually an issue though. Is it possible we could be handling more than one HTTP response for the same operation at once? If not, then I wouldn't worry about it. If we don't know, or if it doesn't happen now but could in the future, we should handle it.

fixing issue

2348639

Paramadon requested a review from a team as a code owner November 12, 2024 20:47

Paramadon added 19 commits November 12, 2024 16:01

trying to fix logs

b7fac5e

fixing issue

45536b8

adding debug statements (swapping logger with log.print)

75b024a

adding more log statments

b41de09

fixing tags

90a22a3

fixing logs

8c6d037

fixing logs

6ff99f8

adding a log

667f608

fixing stats function for statuscodehandler

9de9448

fixing issue

1e690e3

making operation names shorter

4c6adcf

Last commit works to reduce name- this one is for filtering operation…

dc129c9

…s from status codes

adding status code only filter

07a06fa

removing status code from otel config

0f76726

had to add return statments to filter

965ccdb

commenting out try configure for ec2tagger

a8268fd

adding ecw client for ec2 tagger

8ddb463

changing the middleware to metricID

3c15e81

trying to get stat

947b8f3

Paramadon changed the title ~~fixing issue~~ Adding Agent Health Extension Nov 15, 2024

Paramadon added 7 commits November 15, 2024 15:44

everything works just removing some debug statement and filter from h…

a76ef49

…andler

fixing filter issue

abf484b

previous commit works, just cleaning up debug statemtns

c849f61

adding unit tests

bd33dbc

fixing test

822680f

fixing unit tests

d547e1c

running make fmt

5dc40f4

Paramadon force-pushed the CodeHandler branch from 9465711 to 5dc40f4 Compare November 19, 2024 21:51

Paramadon added 7 commits November 19, 2024 17:18

fixing unit tests

e5bf295

fixing tests

e07463a

fixing yamls

a5ba52e

fixing yamls

f3c8d46

removing internal changes

f64b332

restoring files

fa4c163

removing unnecessary file changes and passing unit test

457e48c

Paramadon changed the title ~~Adding Agent Health Extension~~ Adding Agent Health Extension Statuscode handler Nov 19, 2024

Paramadon changed the title ~~Adding Agent Health Extension Statuscode handler~~ Adding Agent Health Extension Statuscode Handler Nov 19, 2024

Paramadon changed the title ~~Adding Agent Health Extension Statuscode Handler~~ Adding Statuscode Handler to Agent Health Extension Nov 19, 2024

Paramadon added 3 commits November 19, 2024 18:12

cleaning up code

2705e87

fixing up pointer issue and passing in operations

3083626

adding agent health to prometheus

9e40bd8

mitali-salvi reviewed Nov 21, 2024

View reviewed changes

musa-asad reviewed Nov 21, 2024

View reviewed changes

tool/clean/clean_host/clean_host.go Outdated Show resolved Hide resolved

dricross reviewed Nov 21, 2024

View reviewed changes

jefchien reviewed Nov 21, 2024

View reviewed changes

Paramadon added 6 commits November 21, 2024 20:01

resolving comments

5745ee6

resolving comments

6e0fdd3

adding tests

8aee0d9

resolving comments

f344518

resolving comments

a7e0415

adding tests

d91706d

dricross reviewed Nov 22, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding Statuscode Handler to Agent Health Extension #1423

Adding Statuscode Handler to Agent Health Extension #1423

Paramadon commented Nov 12, 2024 •

edited

Loading

mitali-salvi left a comment

mitali-salvi Nov 21, 2024

mitali-salvi Nov 21, 2024

jefchien Nov 21, 2024

dricross Nov 21, 2024

Paramadon Nov 21, 2024 •

edited

Loading

jefchien Nov 21, 2024

jefchien Nov 21, 2024

Paramadon Nov 22, 2024

dricross Nov 22, 2024

Adding Statuscode Handler to Agent Health Extension #1423

Are you sure you want to change the base?

Adding Statuscode Handler to Agent Health Extension #1423

Conversation

Paramadon commented Nov 12, 2024 • edited Loading

Description of the Issue

Changes Made

Impact

Testing

mitali-salvi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Paramadon Nov 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Paramadon commented Nov 12, 2024 •

edited

Loading

Paramadon Nov 21, 2024 •

edited

Loading