Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[receiver/dockerstatsreceiver] Add metrics that are only available on hosts that use cgroups v2 #22706

Merged
merged 3 commits into from
Jun 25, 2023
Merged

[receiver/dockerstatsreceiver] Add metrics that are only available on hosts that use cgroups v2 #22706

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c03294f
Add missing metrics for cgroups v2
kang-makes May 23, 2023
0050f6e
improve changelog message
kang-makes May 23, 2023
ccfbeef
recreate mocks
kang-makes Jun 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .chloggen/receiver_dockerstatsreceiver_add-missing-metrics-for-cgroups-v2.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: breaking
Copy link
Contributor

Choose a reason for hiding this comment

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

Just wanted to check why is this marked as breaking?

From what I can tell from the user's perspective these changes are additive?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I know, adding a new metric by default is a breaking change.

I added by default this one: https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/22706/files#diff-9fde4f9bed227e12ddb4ce32989c6acdb00339e533d8b24688bb43456dc433eaR429

And as I wrote on the PR description:

I enabled the metric container.memory.file by default (causing a breaking change) because it is a metric that was already scraped for cgroups v1, contained in the metric container.memory.total_cache.

Let me know if it is not OK and I change it to enhancement.

Copy link
Contributor

Choose a reason for hiding this comment

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

It make sense to me, happy to keep it as breaking.


# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: dockerstatsreceiver

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Add non populated metrics `container.memory.anon` and `container.memory.file` that are only available when the host uses cgroups v2.

# One or more tracking issues related to the change
issues: [ 21097 ]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext: |
Metric `container.memory.file` was added to fulfil the lack of `container.memory.total_cache` metric in case that the
cgroup version mounted in the host is v2 instead of v1.

Now there is documentation of which metrics are available depending cgroups version being used on the host.
88 changes: 54 additions & 34 deletions receiver/dockerstatsreceiver/documentation.md
View file
Open in desktop

Large diffs are not rendered by default.

8 changes: 8 additions & 0 deletions receiver/dockerstatsreceiver/internal/metadata/generated_config.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions receiver/dockerstatsreceiver/internal/metadata/generated_config_test.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

184 changes: 151 additions & 33 deletions receiver/dockerstatsreceiver/internal/metadata/generated_metrics.go
View file
Open in desktop

Large diffs are not rendered by default.

101 changes: 68 additions & 33 deletions receiver/dockerstatsreceiver/internal/metadata/generated_metrics_test.go
View file
Open in desktop

Large diffs are not rendered by default.

8 changes: 8 additions & 0 deletions receiver/dockerstatsreceiver/internal/metadata/testdata/config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,14 @@ all_set:
enabled: true
container.memory.active_file:
enabled: true
container.memory.anon:
enabled: true
container.memory.cache:
enabled: true
container.memory.dirty:
enabled: true
container.memory.file:
enabled: true
container.memory.hierarchical_memory_limit:
enabled: true
container.memory.hierarchical_memsw_limit:
Expand Down Expand Up @@ -184,10 +188,14 @@ none_set:
enabled: false
container.memory.active_file:
enabled: false
container.memory.anon:
enabled: false
container.memory.cache:
enabled: false
container.memory.dirty:
enabled: false
container.memory.file:
enabled: false
container.memory.hierarchical_memory_limit:
enabled: false
container.memory.hierarchical_memsw_limit:
Expand Down
Loading