Skip to content

Add container statuses#61

Merged
unflxw merged 4 commits into
mainfrom
add-container-statuses
Jul 23, 2025
Merged

Add container statuses#61
unflxw merged 4 commits into
mainfrom
add-container-statuses

Conversation

@unflxw

@unflxw unflxw commented Jul 18, 2025

Copy link
Copy Markdown
Contributor

See Slack conversation for context. Also see protocol PR.

Report container statuses

For each container in a pod, report the name of the container, its
status, and the reason and exit code for said status, if any.

Report metrics when pods have no stats

When a pod metric has no stats (CPU, memory... as reported by the node
stats summary endpoint) report it anyway. Do this by storing the
pods stats while traversing the stats JSON, but emitting metrics for
all pods, as reported by the Kubernetes API, not just for those
listed in the stats endpoint.

Fixes #57.

@unflxw unflxw self-assigned this Jul 18, 2025
@unflxw
unflxw requested review from matsimitsu and tombruijn July 18, 2025 21:34
@backlog-helper

backlog-helper Bot commented Jul 18, 2025

Copy link
Copy Markdown

✔️ All good!

New issue guide | Backlog management | Rules | Feedback

@unflxw
unflxw force-pushed the add-container-statuses branch from e84284c to dab9b8e Compare July 18, 2025 21:53
@unflxw
unflxw force-pushed the add-container-statuses branch from 74583cb to 23bb6f7 Compare July 21, 2025 20:20
@unflxw

unflxw commented Jul 21, 2025

Copy link
Copy Markdown
Contributor Author

@tombruijn FYI: I've rolled in the changes re: using enums (which I discussed with @matsimitsu over in Slack) into this PR, for both container statuses and pod phases.

Note that the protocol is backwards-compatible for phases, but not for container statuses, since those haven't been released yet. The implementation, however, isn't backwards-compatible for phases -- the processor should attempt to read pod_phase (the enum), and if that fails, attempt to read phase (the string)

@matsimitsu

matsimitsu commented Jul 22, 2025

Copy link
Copy Markdown
Member

the processor should attempt to read pod_phase (the enum), and if that fails, attempt to read phase (the string)

That's not possible, the enum is always set (just to the default value), so I'm currently reading the deprecated phase, and if it's Unknown or empty, then I read the new pod_phase field.

@backlog-helper

This comment has been minimized.

@unflxw

unflxw commented Jul 22, 2025

Copy link
Copy Markdown
Contributor Author

That's not possible, the enum is always set (just to the default value)

Yeah, sorry, by "fails" I meant "is the default value", which is _UNKNOWN. I know "unknown" is one of the possible phase values, but I thought it could double as the default value -- maybe not, I can add an _UNSPECIFIED as the default if that helps.

@unflxw
unflxw force-pushed the add-container-statuses branch 2 times, most recently from 3cd16b2 to 96df30c Compare July 22, 2025 11:07

@tombruijn tombruijn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great to see it's now sending in batches too!

@backlog-helper

Copy link
Copy Markdown

This is a message from the daily scheduled checks.

New issue guide | Backlog management | Rules | Feedback

@unflxw unflxw added the enhancement An improvement to an existing feature. label Jul 23, 2025
unflxw added 4 commits July 23, 2025 16:40
For each container in a pod, report the name of the container, its
status, and the reason and exit code for said status, if any.
When a pod metric has no stats (CPU, memory... as reported by the node
stats summary endpoint) report it anyway. Do this by storing the
pods stats while traversing the stats JSON, but emitting metrics for
all pods, as reported by the Kubernetes API, not just for those
listed in the stats endpoint.
Report the pod phase and container status as enums. Do not emit the
previous `phase` string property in the KubernetesMetrics struct,
which is now deprecated. Instead, emit the `pod_phase` enum property.

Replace the not-yet-released `status` string property with an `enum`
property of the same name.
Instead of sending one request to AppSignal for each node, pod and
volume in the Kubernetes cluster, send several metrics in each
request.

Set a threshold for request size of 400 kilobytes. It's not strictly
a maximum, as it will be exceeded if a single metric somehow exceeds
it.
@unflxw
unflxw force-pushed the add-container-statuses branch from 96df30c to c7b13ca Compare July 23, 2025 14:40
@unflxw
unflxw merged commit 98d5e48 into main Jul 23, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement An improvement to an existing feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Report pod status

3 participants