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.

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

[receiver/dockerstats] Change cpu utilization metric #22159

Conversation

carlossscastro
Copy link
Contributor

Description:
This change starts the process for deprecating the metric container.cpu.percent in favor of container.cpu.utilization to adhere to semantic convention.
New metric naming is under a feature-gate that can be enabled with:

./otelcontribcol --feature-gates=+receiver.dockerstats.changeCpuUtilization

Link to tracking Issue: #21807

Testing:

❯ go clean -testcache
❯ make test
go test -race -timeout 300s -parallel 4 --tags="" ./...
ok  	github.com/open-telemetry/opentelemetry-collector-contrib/receiver/dockerstatsreceiver	0.829s
ok  	github.com/open-telemetry/opentelemetry-collector-contrib/receiver/dockerstatsreceiver/internal/metadata	0.375s
❯ go test --tags integration ./...
ok  	github.com/open-telemetry/opentelemetry-collector-contrib/receiver/dockerstatsreceiver	9.873s
ok  	github.com/open-telemetry/opentelemetry-collector-contrib/receiver/dockerstatsreceiver/internal/metadata	0.235s

Documentation:
Metrics documentation was updated using mdatagen

@carlossscastro carlossscastro requested review from a team and Aneurysm9 May 22, 2023 15:52
@carlossscastro carlossscastro changed the title Change cpu utilization metric [receiver/dockerstats] Change cpu utilization metric May 22, 2023
@andrzej-stencel
Copy link
Member

@carlossscastro sorry I should have thought about this and mentioned this before: with a metric name change like this, we should be able to get away without a feature gate, instead using the regular metric enabling/disabling capabilities and a description of the deprecation in the README file. See similar issue and PRs in the Host Metrics receiver:

Do you think we can do it this way? Again, very sorry for bringing this up after you implemented the feature gate. 😟

MovieStoreGuy and others added 11 commits May 23, 2023 11:15
 (open-telemetry#17020)

* [schema processor] Modifier

A modifier allows mutating a signal from a previous version to the
current one and vice versa.
[schema processor] Adding revisions

A revision handles applying the modifiers to incoming signals and
converting a signal to the next version.

* Adding changes

* Adding migrate package.

Following patterns similar to databases where you'd apply changes
and rollback. the Migrate packages follows these designs to help make it
clear what it is trying to do.

* Removing modiferies to avoid confusion

* Fixing up naming issues

* Adding checks in for building a revision

* Fixing up replaces

* Removing transform files

* Fixing naming

* Fixing up conflict

* Fixing typo

* Updating naming

* Fixing up ast usage

* FIxing up generated mod files

* Fixing up license headers
…etry#22700)

* [exporter/datasetexporter]: Enable in internal components

* Add changelog entry

* Modify go.mod and go.sum files
…lemetry#22059)

* Add IntLikeGetter

* changelog

* fix lint

* Update pkg/ottl/expression_test.go

Co-authored-by: Evan Bradley <[email protected]>

* Update pkg/ottl/expression_test.go

Co-authored-by: Evan Bradley <[email protected]>

---------

Co-authored-by: Evan Bradley <[email protected]>
Corrected some invalid docs in README:
- This receiver does not use gRPC, replaced with HTTP config
- This receiver does not use queuing, retry and timeout settings from exporterhelper
…g keys (open-telemetry#22708)

Allow capture groups in replace_all_patterns when using the key mode

Co-authored-by: Evan Bradley <[email protected]>
@carlossscastro
Copy link
Contributor Author

Do you think we can do it this way? Again, very sorry for bringing this up after you implemented the feature gate. 😟

No problem at all @astencel-sumo. I will work on this and resubmit the PR. Thanks again for the guidance.

@atoulme
Copy link
Contributor

atoulme commented May 26, 2023

@carlossscastro please take a look at the PR, I think you have had a weird push.

@carlossscastro
Copy link
Contributor Author

Resubmitting in a new PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.