Apply new experimental metrics builder to hostmetrics/cpu scraper #6418
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dmitryax
changed the title
dmitryax
force-pushed
the
add-exp-metadata-gen
branch
2 times, most recently
from
a3444d5
to
01b3521
Compare
bogdandrutu
reviewed
Nov 23, 2021
receiver/hostmetricsreceiver/internal/scraper/cpuscraper/internal/metadata/generated_metrics.go
Outdated
Show resolved
Hide resolved
receiver/hostmetricsreceiver/internal/scraper/cpuscraper/internal/metadata/generated_metrics.go
Outdated
Show resolved
Hide resolved
receiver/hostmetricsreceiver/internal/scraper/cpuscraper/internal/metadata/generated_metrics.go
Outdated
Show resolved
Hide resolved
bogdandrutu
reviewed
Nov 23, 2021
dmitryax
commented
Nov 23, 2021
bogdandrutu
reviewed
Nov 29, 2021
receiver/hostmetricsreceiver/internal/scraper/cpuscraper/cpu_scraper.go
Outdated
Show resolved
Hide resolved
receiver/hostmetricsreceiver/internal/scraper/cpuscraper/cpu_scraper.go
Outdated
Show resolved
Hide resolved
receiver/hostmetricsreceiver/internal/scraper/cpuscraper/cpu_scraper.go
Outdated
Show resolved
Hide resolved
bogdandrutu
reviewed
Nov 29, 2021
receiver/hostmetricsreceiver/internal/scraper/cpuscraper/cpu_scraper.go
Outdated
Show resolved
Hide resolved
receiver/hostmetricsreceiver/internal/scraper/cpuscraper/cpu_scraper_linux.go
Outdated
Show resolved
Hide resolved
djaglowski
approved these changes
Nov 30, 2021
| withStartTime := metadata.WithStartTime(pdata.Timestamp(bootTime * 1e9)) | ||
| withStatesCapacity := metadata.WithAttributeStateCapacity(cpuStatesLen) | ||
| withCoresCapacity := metadata.WithAttributeCpuCapacity(runtime.NumCPU()) | ||
| s.mb = metadata.NewMetricsBuilder(s.config.Metrics, withStartTime, withStatesCapacity, withCoresCapacity) |
There was a problem hiding this comment.
Some scrapers will have many more "Capacity" parameters. Should we render these into separate functions, perhaps like this?
s.mb = metadata.NewMetricsBuilder(s.config.Metrics, withStartTime).
WithStatesCapacity(withStatesCapacity).
WithCoresCapacity(withCoresCapacity)There was a problem hiding this comment.
In that case I would change it for all including withStartTime. I like it, but seems like it's not very common pattern in the collector's codebase. @bogdandrutu what's your thought on this?
There was a problem hiding this comment.
I think this is the most "go" way to have this:
s.mb = metadata.NewMetricsBuilder(
s.config.Metrics,
metadata.WithStartTime(pdata.Timestamp(bootTime * 1e9)),
metadata.WithAttributeStateCapacity(cpuStatesLen),
metadata.WithAttributeCpuCapacity(runtime.NumCPU()))
receiver/hostmetricsreceiver/internal/scraper/cpuscraper/cpu_scraper.go
Outdated
Show resolved
Hide resolved
dmitryax
force-pushed
the
add-exp-metadata-gen
branch
5 times, most recently
from
336f0f5
to
505d64a
Compare
Introduce new experimental metrics builder interface for metrics scrapers and apply it to hostmetrics/cpu scraper.
dmitryax
force-pushed
the
add-exp-metadata-gen
branch
from
505d64a
to
21114a8
Compare
bogdandrutu
reviewed
Dec 2, 2021
| func (mb *MetricsBuilder) Emit(metrics pdata.MetricSlice) { | ||
| {{- range $name, $metric := .Metrics }} | ||
| if mb.config.{{- $name.Render }}.Enabled { | ||
| mb.metrics.{{- $name.Render }}.CopyTo(metrics.AppendEmpty()) |
jamesmoessis
pushed a commit
to atlassian-forks/opentelemetry-collector-contrib
that referenced
this pull request
Dec 8, 2021
…pen-telemetry#6418) Introduce new experimental metrics builder interface for metrics scrapers and apply it to hostmetrics/cpu scraper.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduce new experimental metrics builder interface for metrics scrapers and apply it to hostmetrics/cpu scraper.
Link to tracking Issue: open-telemetry/opentelemetry-collector#10904