[podmanreceiver] Add metrics and resource metadata #30232

rogercoll · 2023-12-28T15:56:37Z

Description:

Adds "metadata.yml" file to autogenerate metrics and resources.
[Update: not done in this PR] Fixes invalid network metrics: "rx -> input" and "tx -> output"

Link to tracking Issue: #28640

Testing: Previous tests preserved.

Documentation:

fatsheep9146 · 2023-12-29T10:17:23Z

receiver/podmanreceiver/metadata.yaml

+  container.blockio.io_service_bytes_recursive.read:
+    enabled: true
+    description: "Number of bytes transferred from the disk by the container"
+    extended_documentation: "[More docs]i/www.kernel.org/doc/Documentation/cgroup-v1/blkio-controller.txt)."


It seems that the format of link is not valid. Should it be like this? [More docs](https://www.kernel.org/doc/Documentation/cgroup-v1/blkio-controller.txt).

Fixed in df281be

fatsheep9146 · 2023-12-29T10:20:37Z

Could you also update the metric section in ReadMe.md to let user to see the documentation.md to check about the metrics supported by podman receiver?

fatsheep9146 · 2023-12-29T10:27:36Z

receiver/podmanreceiver/metadata.yaml

+  container.memory.usage.limit:
+    enabled: true
+    description: "Memory limit of the container."
+    unit: 1


I think the memory related metrics' unit should be By.

You can take https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/hostmetricsreceiver/internal/scraper/memoryscraper/metadata.yaml as example

Makes sense, fixed in df281be

rogercoll · 2023-12-29T17:05:56Z

Could you also update the metric section in ReadMe.md to let user to see the documentation.md to check about the metrics supported by podman receiver?

Definitely, thanks for the feedback. Added in df281be

fatsheep9146 · 2023-12-31T08:45:01Z

receiver/podmanreceiver/metadata.yaml

+  container.blockio.io_service_bytes_recursive.write:
+    enabled: true
+    description: "Number of bytes transferred to the disk by the container"
+    extended_documentation: "[More docs]i/www.kernel.org/doc/Documentation/cgroup-v1/blkio-controller.txt)."


This line should also be fixed

Fixed in 5f3528c. Thanks

fatsheep9146 · 2024-01-02T04:30:02Z

receiver/podmanreceiver/metadata.yaml

+  container.cpu.usage.system:
+    enabled: true
+    description: "System CPU usage."
+    unit: ns


https://opentelemetry.io/docs/specs/semconv/system/system-metrics/#metric-systemcputime
I'm not sure if cputime unit should be ns or s, @open-telemetry/collector-approvers should this metric use s as unit?

I think that cputime metric's unit is in seconds, as this is how many OSes report it (/proc/stats). But containers are handled by cgroup controllers, which allow for better precision. I would say not to lose precision, instead add container metrics into the semantic convention.

Your comments also make sense, I will pull this discussion to slacks, to get more input. @rogercoll

The docker stats receiver collects container.cpu.usage.system as well: https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/dockerstatsreceiver/metadata.yaml#L63-L67, and the unit is ns. IMO we should also use ns here to stay consistent between the two.

The docker stats receiver collects container.cpu.usage.system as well: https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/dockerstatsreceiver/metadata.yaml#L63-L67, and the unit is ns. IMO we should also use ns here to stay consistent between the two.

Make senses, I already throw this to other approvers in slack, if they have no objection to this, I will approve this pr.
@rogercoll @mackjmr

Sounds good! Does it make sense to start working on the container's semantic convention PR? (feel free to add me into the slack thread @rogercoll)

No one responses for that for a day. And as @mackjmr said docker stats receiver also take ns as unit, so I think this is ok to approve.

cmd/mdatagen/validate_test.go

Co-authored-by: Mackenzie <63265430+mackjmr@users.noreply.github.com>

mx-psi · 2024-01-29T16:22:06Z

cmd/mdatagen/validate_test.go

+		"container.cpu.utilization":           {"docker_stats", "kubeletstats"},
+		"container.cpu.usage.system":          {"docker_stats", "podman_stats"},
+		"container.cpu.usage.percpu":          {"docker_stats", "podman_stats"},
+		"container.cpu.usage.total":           {"docker_stats", "podman_stats"},


Is it okay to report the same metric with different units?

As discussed above, on the Docker stats receiver we have nanoseconds

opentelemetry-collector-contrib/receiver/dockerstatsreceiver/documentation.md

Lines 41 to 47 in facd369

### container.cpu.usage.total

Total CPU time consumed.

| Unit | Metric Type | Value Type | Aggregation Temporality | Monotonic |

| ---- | ----------- | ---------- | ----------------------- | --------- |

| ns | Sum | Int | Cumulative | true |

while here we use seconds

opentelemetry-collector-contrib/receiver/podmanreceiver/documentation.md

Lines 65 to 71 in 91e24ff

### container.cpu.usage.total

Total CPU time consumed.

| Unit | Metric Type | Value Type | Aggregation Temporality | Monotonic |

| ---- | ----------- | ---------- | ----------------------- | --------- |

| s | Sum | Int | Cumulative | true |

Is that okay? If seconds is the right unit, shouldn't we use it on the Docker stats receiver as well?

I don't think so, I would not use second's precision for the sake of convenience at the expense of precision. Instead, we should endeavor to establish distinct conventions specifically tailored to containers. Should we wait for the container's semantic convention open-telemetry/semantic-conventions#282 (nanoseconds)?

I don't think so, I would not use second's precision for the sake of convenience at the expense of precision. Instead, we should endeavor to establish distinct conventions specifically tailored to containers.

My objection here is with the use of different units on each metric, I would expect them to have the same unit (whether it is nanoseconds or seconds I agree is something we can leave open-telemetry/semantic-conventions#282 to decide on)

github-actions · 2024-02-17T05:18:42Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

MovieStoreGuy · 2024-03-04T02:08:38Z

Can I ask that the files in conflict get fixed up, outside of that, I don't see any outstanding issues that would block this PR?

fatsheep9146 · 2024-03-04T04:37:36Z

Can I ask that the files in conflict get fixed up, outside of that, I don't see any outstanding issues that would block this PR?

I think we still wait for the open-telemetry/semantic-conventions#282 to be merged first

@MovieStoreGuy

github-actions · 2024-03-18T05:19:55Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

fatsheep9146 · 2024-03-18T05:31:06Z

just ping jsuereth on open-telemetry/semantic-conventions#282 to merge the pr.

fatsheep9146 · 2024-03-27T12:54:31Z

@rogercoll could you push forward this pr since open-telemetry/semantic-conventions#282 is merge?

rogercoll · 2024-03-28T16:39:59Z

MovieStoreGuy · 2024-04-09T08:13:17Z

My preference would be to split the changes to keep change sizes small, are you okay with that @fatsheep9146 ?

fatsheep9146 · 2024-04-09T08:50:44Z

My preference would be to split the changes to keep change sizes small, are you okay with that @fatsheep9146 ?

I agreed. @rogercoll could you fix the conflicts, so I could push this pr to be merged ASAP.

rogercoll · 2024-04-09T10:56:33Z

I agreed. @rogercoll could you fix the conflicts, so I could push this pr to be merged ASAP.

Done, I also updated the changelog file to a "breaking" change type due to the cpu precision fix (ns -> s).

rogercoll · 2024-04-09T10:58:17Z

If we prefer a totally no-op change PR, I could rollback the a5a0099 commit and push it into a different PR. Let me know what you think

rogercoll added 2 commits December 28, 2023 16:45

feat: add Podman receiver metadata

cbeaf84

docs: add change log file

b9a3406

rogercoll requested review from a team and fatsheep9146 December 28, 2023 15:56

github-actions bot assigned MovieStoreGuy Dec 28, 2023

github-actions bot added receiver/dockerstats receiver/podman labels Dec 28, 2023

rogercoll added 4 commits December 28, 2023 17:05

fix: gci linter

78901ea

fix: tidy module file

64c0efc

fix: spelling mistake

5455ff6

fix: allow container duplicated metrics

b9935f8

rogercoll requested a review from dmitryax as a code owner December 28, 2023 16:48

github-actions bot added the cmd/mdatagen mdatagen command label Dec 28, 2023

fatsheep9146 reviewed Dec 29, 2023

View reviewed changes

fix: use "By" unit type for memory metrics

df281be

Merge branch 'main' into add_podman_metadata

d528755

fatsheep9146 reviewed Dec 31, 2023

View reviewed changes

docs: fix blkio metric kernel documentation url

5f3528c

fatsheep9146 reviewed Jan 2, 2024

View reviewed changes

mackjmr reviewed Jan 3, 2024

View reviewed changes

cmd/mdatagen/validate_test.go Outdated Show resolved Hide resolved

rogercoll and others added 2 commits January 3, 2024 17:21

Update cmd/mdatagen/validate_test.go

befc88c

Co-authored-by: Mackenzie <63265430+mackjmr@users.noreply.github.com>

Merge branch 'main' into add_podman_metadata

6c37699

fatsheep9146 approved these changes Jan 4, 2024

View reviewed changes

mackjmr approved these changes Jan 5, 2024

View reviewed changes

rogercoll added 3 commits January 9, 2024 08:45

Merge branch 'main' into add_podman_metadata

0cbdc4e

Merge branch 'main' into add_podman_metadata

1bd427e

Merge branch 'main' into add_podman_metadata

0032f70

rollback fix for networking tx/rx metrics

91e24ff

mx-psi reviewed Jan 29, 2024

View reviewed changes

github-actions bot added the Stale label Feb 17, 2024

fatsheep9146 removed the Stale label Feb 20, 2024

rogercoll mentioned this pull request Mar 13, 2024

[receiver/podman] Add functionalities to Podman based on Docker stats receiver #9013

Closed

5 tasks

github-actions bot added the Stale label Mar 18, 2024

fatsheep9146 removed the Stale label Mar 18, 2024

rogercoll added 3 commits March 27, 2024 17:04

Merge branch 'main' into add_podman_metadata

b2ed9a8

Merge branch 'main' into add_podman_metadata

fa12570

Merge branch 'main' into add_podman_metadata

6d89c48

rogercoll added 3 commits April 9, 2024 12:49

Merge branch 'main' into add_podman_metadata

70d142c

update changelog with breaking changes

384cb7d

Merge branch 'main' into add_podman_metadata

0dfb234

MovieStoreGuy merged commit fe59abb into open-telemetry:main Apr 9, 2024
169 of 170 checks passed

github-actions bot added this to the next release milestone Apr 9, 2024

rogercoll deleted the add_podman_metadata branch April 10, 2024 09:06

rogercoll mentioned this pull request May 6, 2024

[receiver/podmanreceiver] Add metrics in metadata.yaml #28640

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[podmanreceiver] Add metrics and resource metadata #30232

[podmanreceiver] Add metrics and resource metadata #30232

rogercoll commented Dec 28, 2023 •

edited

Loading

fatsheep9146 Dec 29, 2023 •

edited

Loading

rogercoll Dec 29, 2023

fatsheep9146 commented Dec 29, 2023

fatsheep9146 Dec 29, 2023

fatsheep9146 Dec 29, 2023

rogercoll Dec 29, 2023

rogercoll commented Dec 29, 2023

fatsheep9146 Dec 31, 2023

rogercoll Dec 31, 2023

fatsheep9146 Jan 2, 2024

rogercoll Jan 4, 2024

fatsheep9146 Jan 4, 2024

mackjmr Jan 4, 2024

fatsheep9146 Jan 4, 2024

rogercoll Jan 4, 2024

fatsheep9146 Jan 4, 2024

mx-psi Jan 29, 2024 •

edited

Loading

rogercoll Feb 1, 2024

mx-psi Feb 2, 2024

github-actions bot commented Feb 17, 2024

MovieStoreGuy commented Mar 4, 2024

fatsheep9146 commented Mar 4, 2024 •

edited

Loading

github-actions bot commented Mar 18, 2024

fatsheep9146 commented Mar 18, 2024

fatsheep9146 commented Mar 27, 2024

rogercoll commented Mar 28, 2024 •

edited

Loading

MovieStoreGuy commented Apr 9, 2024

fatsheep9146 commented Apr 9, 2024

rogercoll commented Apr 9, 2024

rogercoll commented Apr 9, 2024

	### container.cpu.usage.total

	Total CPU time consumed.

	\| Unit \| Metric Type \| Value Type \| Aggregation Temporality \| Monotonic \|
	\| ---- \| ----------- \| ---------- \| ----------------------- \| --------- \|
	\| ns \| Sum \| Int \| Cumulative \| true \|

[podmanreceiver] Add metrics and resource metadata #30232

[podmanreceiver] Add metrics and resource metadata #30232

Conversation

rogercoll commented Dec 28, 2023 • edited Loading

fatsheep9146 Dec 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fatsheep9146 commented Dec 29, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rogercoll commented Dec 29, 2023

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

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

mx-psi Jan 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 17, 2024

MovieStoreGuy commented Mar 4, 2024

fatsheep9146 commented Mar 4, 2024 • edited Loading

github-actions bot commented Mar 18, 2024

fatsheep9146 commented Mar 18, 2024

fatsheep9146 commented Mar 27, 2024

rogercoll commented Mar 28, 2024 • edited Loading

MovieStoreGuy commented Apr 9, 2024

fatsheep9146 commented Apr 9, 2024

rogercoll commented Apr 9, 2024

rogercoll commented Apr 9, 2024

rogercoll commented Dec 28, 2023 •

edited

Loading

fatsheep9146 Dec 29, 2023 •

edited

Loading

mx-psi Jan 29, 2024 •

edited

Loading

fatsheep9146 commented Mar 4, 2024 •

edited

Loading

rogercoll commented Mar 28, 2024 •

edited

Loading