Add ComponentHealth.attributes#334
Conversation
tigrannajaryan
left a comment
tigrannajaryan
left a comment
There was a problem hiding this comment.
What are "attributes" supposed to carry? We need a more elaborate motivation before accepting this change.
Looking at the extension event, it says "attributes provides additional context or metadata for the event" which is very vague to me.
|
For example the attributes of a hostmetrics scraper can include things to indicate specifics on which scrapers are failing, as seen in the extension issue is: "pipeline:metrics/1": {
"healthy": true,
"status": "StatusRecoverableError",
"status_time": "2025-10-16T17:56:11.820542393+02:00",
"attributes": {
"error_msg": "not enough permissions to read cpu data",
"scrapers": ["cpu", "memory", "network"]
},It could further be used to indicate other metadata such as port numbers, address, etc. |
tigrannajaryan
left a comment
tigrannajaryan
left a comment
There was a problem hiding this comment.
@michel-laterman sorry for late review. I think it makes sense for us to mirror the structure of the status that the Collector has.
|
I will keep this open for a while in case @andykellr or @evan-bradley have comments. |
There was a problem hiding this comment.
Pull request overview
Extends the OpAMP ComponentHealth message to include attributes, aligning the spec and protobuf definition with the metadata produced by the OpenTelemetry Collector healthcheck/componentstatus model (Issue #326).
Changes:
- Added
ComponentHealth.attributesto the specification, including TOC entry and field documentation. - Added
repeated KeyValue attributes = 7;toComponentHealthinproto/opamp.protowith matching comments.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| specification.md | Documents the new ComponentHealth.attributes field and adds it to the ComponentHealth section/TOC. |
| proto/opamp.proto | Adds the attributes field to the ComponentHealth protobuf message (field number 7). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // Additional context or metadata for the observed component's status. | ||
| // Attributes are component specific and outside the concerns of the OpAMP protocol. | ||
| repeated KeyValue attributes = 7; |
There was a problem hiding this comment.
Please mark with // Status: [Development] (see other similar fields in this proto).
|
Please resolve the conflict. |
Add
attributesin order to be consistent with what the healthcheck extension produces. See extension source.