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/prometheus: add metricGroup.toNumberDataPoint pdata conversion #3674

Conversation

odeke-em
Copy link
Member

Implements metricGroupPdata toNumberDataPoint and added unit tests
as well as equivalence tests to ensure the migration will render
the same results.

Updates #3137
Depends on PR #3668
Updates PR #3427

@odeke-em odeke-em requested a review from alolita as a code owner July 19, 2021 06:00
@odeke-em odeke-em requested review from a team and dmitryax July 19, 2021 06:00
@alolita
Copy link
Member

alolita commented Jul 19, 2021

@dashpole @Aneurysm9 @rakyll please review.

Copy link
Contributor

@dashpole dashpole left a comment

Choose a reason for hiding this comment

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

toNumberDataPoint implementation and testing looks good.

@punya
Copy link
Member

punya commented Jul 20, 2021

@odeke-em could you please appease the linter:

receiver/prometheusreceiver/internal/otlp_metricfamily_test.go:509: File is not `gofmt`-ed with `-s` (gofmt)
        type scrape struct {

@odeke-em odeke-em force-pushed the receiver-prometheus-metricGroupToNumberDataPointPdata-migration branch from 1d422c6 to 0d94cb4 Compare July 21, 2021 03:29
@tigrannajaryan
Copy link
Member

@Aneurysm9 @rakyll do you plan to review? If not I can merge based on @dashpole's approval.

@odeke-em odeke-em force-pushed the receiver-prometheus-metricGroupToNumberDataPointPdata-migration branch from 0d94cb4 to 2606ea6 Compare July 21, 2021 17:44
@odeke-em
Copy link
Member Author

odeke-em commented Jul 21, 2021

Thanks for the reviews @punya @dashpole @tigrannajaryan! @bogdandrutu this one too is ready :-)

@punya punya added the ready-to-merge Code review completed; ready to merge by maintainers label Jul 21, 2021
@bogdandrutu bogdandrutu removed the ready-to-merge Code review completed; ready to merge by maintainers label Jul 21, 2021
@odeke-em odeke-em force-pushed the receiver-prometheus-metricGroupToNumberDataPointPdata-migration branch from 2606ea6 to ef882b9 Compare July 21, 2021 23:33
@odeke-em odeke-em requested a review from bogdandrutu July 21, 2021 23:35
Implements metricGroupPdata toNumberDataPoint and added unit tests
as well as equivalence tests to ensure the migration will render
the same results.

While here, added TODOs for issue open-telemetry#3691 which found a bug in
which cumulative types weren't using the actual duration start
timestamp. Given that this current change is a translation of prior
logic and has parity checks, making that bug fix would complicate
the PR.

Updates #3137
Depends on PR open-telemetry#3668
Updates PR open-telemetry#3427
Updates open-telemetry#3691
@odeke-em odeke-em force-pushed the receiver-prometheus-metricGroupToNumberDataPointPdata-migration branch from ef882b9 to 2ab79f2 Compare July 22, 2021 02:11
odeke-em added a commit to orijtech/opentelemetry-collector that referenced this pull request Jul 22, 2021
This change wires together the logic to convert
the pdata converters into a method ToMetricPdata
that appends converted metrics. The next change or two,
will then hook everything up directly and will
allow us to delete all the prior code!!

Updates #3137
Updates PR open-telemetry#3674
Requires PR open-telemetry#3694
Updates open-telemetry#3691
odeke-em added a commit to orijtech/opentelemetry-collector that referenced this pull request Jul 22, 2021
This change wires together the logic to convert
the pdata converters into a method ToMetricPdata
that appends converted metrics. The next change or two,
will then hook everything up directly and will
allow us to delete all the prior code!!

Updates #3137
Updates PR open-telemetry#3674
Requires PR open-telemetry#3694
Updates open-telemetry#3691
odeke-em added a commit to orijtech/opentelemetry-collector that referenced this pull request Jul 22, 2021
This change wires together the logic to convert
the pdata converters into a method ToMetricPdata
that appends converted metrics. The next change or two,
will then hook everything up directly and will
allow us to delete all the prior code!!

Updates #3137
Updates PR open-telemetry#3674
Requires PR open-telemetry#3694
Updates open-telemetry#3691
@bogdandrutu bogdandrutu merged commit 4c92ef4 into open-telemetry:main Jul 22, 2021
@odeke-em odeke-em deleted the receiver-prometheus-metricGroupToNumberDataPointPdata-migration branch July 22, 2021 16:08
odeke-em added a commit to orijtech/opentelemetry-collector that referenced this pull request Jul 23, 2021
This change wires together the logic to convert
the pdata converters into a method ToMetricPdata
that appends converted metrics. The next change or two,
will then hook everything up directly and will
allow us to delete all the prior code!!

Updates #3137
Updates PR open-telemetry#3674
Requires PR open-telemetry#3694
Updates open-telemetry#3691
odeke-em added a commit to orijtech/opentelemetry-collector that referenced this pull request Jul 29, 2021
This change wires together the logic to convert
the pdata converters into a method ToMetricPdata
that appends converted metrics. The next change or two,
will then hook everything up directly and will
allow us to delete all the prior code!!

Updates #3137
Updates PR open-telemetry#3674
Requires PR open-telemetry#3694
Updates open-telemetry#3691
odeke-em added a commit to orijtech/opentelemetry-collector that referenced this pull request Jul 29, 2021
This change wires together the logic to convert
the pdata converters into a method ToMetricPdata
that appends converted metrics. The next change or two,
will then hook everything up directly and will
allow us to delete all the prior code!!

Updates #3137
Updates PR open-telemetry#3674
Requires PR open-telemetry#3694
Updates open-telemetry#3691
odeke-em added a commit to orijtech/opentelemetry-collector that referenced this pull request Jul 29, 2021
This change wires together the logic to convert
the pdata converters into a method ToMetricPdata
that appends converted metrics. The next change or two,
will then hook everything up directly and will
allow us to delete all the prior code!!

Updates #3137
Updates PR open-telemetry#3674
Requires PR open-telemetry#3694
Updates open-telemetry#3691
odeke-em added a commit to orijtech/opentelemetry-collector that referenced this pull request Jul 30, 2021
This change wires together the logic to convert
the pdata converters into a method ToMetricPdata
that appends converted metrics. The next change or two,
will then hook everything up directly and will
allow us to delete all the prior code!!

Updates #3137
Updates PR open-telemetry#3674
Requires PR open-telemetry#3694
Updates open-telemetry#3691
odeke-em added a commit to orijtech/opentelemetry-collector that referenced this pull request Jul 30, 2021
This change wires together the logic to convert
the pdata converters into a method ToMetricPdata
that appends converted metrics. The next change or two,
will then hook everything up directly and will
allow us to delete all the prior code!!

Updates #3137
Updates PR open-telemetry#3674
Requires PR open-telemetry#3694
Updates open-telemetry#3691
odeke-em added a commit to orijtech/opentelemetry-collector that referenced this pull request Jul 30, 2021
This change wires together the logic to convert
the pdata converters into a method ToMetricPdata
that appends converted metrics. The next change or two,
will then hook everything up directly and will allow us
to delete all the prior code!!

Updates #3137
Updates PR open-telemetry#3674
Requires PR open-telemetry#3694
Updates open-telemetry#3691
odeke-em added a commit to orijtech/opentelemetry-collector that referenced this pull request Jul 30, 2021
This change wires together the logic to convert
the pdata converters into a method ToMetricPdata
that appends converted metrics. The next change or two,
will then hook everything up directly and will
allow us to delete all the prior code!!

Updates #3137
Updates PR open-telemetry#3674
Requires PR open-telemetry#3694
Updates open-telemetry#3691
odeke-em added a commit to orijtech/opentelemetry-collector that referenced this pull request Jul 30, 2021
This change wires together the logic to convert
the pdata converters into a method ToMetricPdata
that appends converted metrics. The next change or two,
will then hook everything up directly and will
allow us to delete all the prior code!!

Updates #3137
Updates PR open-telemetry#3674
Requires PR open-telemetry#3694
Updates open-telemetry#3691
odeke-em added a commit to orijtech/opentelemetry-collector that referenced this pull request Jul 31, 2021
This change wires together the logic to convert
the pdata converters into a method ToMetricPdata
that appends converted metrics. The next change or two,
will then hook everything up directly and will
allow us to delete all the prior code!!

Updates #3137
Updates PR open-telemetry#3674
Requires PR open-telemetry#3694
Updates open-telemetry#3691
odeke-em added a commit to orijtech/opentelemetry-collector that referenced this pull request Jul 31, 2021
This change wires together the logic to convert
the pdata converters into a method ToMetricPdata
that appends converted metrics. The next change or two,
will then hook everything up directly and will
allow us to delete all the prior code!!

Updates #3137
Updates PR open-telemetry#3674
Requires PR open-telemetry#3694
Updates open-telemetry#3691
odeke-em added a commit to orijtech/opentelemetry-collector that referenced this pull request Aug 2, 2021
This change wires together the logic to convert
the pdata converters into a method ToMetricPdata
that appends converted metrics. The next change or two,
will then hook everything up directly and will allow us
to delete all the prior code!!

Updates #3137
Updates PR open-telemetry#3674
Requires PR open-telemetry#3694
Updates open-telemetry#3691
odeke-em added a commit to orijtech/opentelemetry-collector that referenced this pull request Aug 2, 2021
This change wires together the logic to convert
the pdata converters into a method ToMetricPdata
that appends converted metrics. The next change or two,
will then hook everything up directly and will allow us
to delete all the prior code!!

Updates #3137
Updates PR open-telemetry#3674
Requires PR open-telemetry#3694
Updates open-telemetry#3691
bogdandrutu pushed a commit that referenced this pull request Aug 3, 2021
This change wires together the logic to convert
the pdata converters into a method ToMetricPdata
that appends converted metrics. The next change or two,
will then hook everything up directly and will allow us
to delete all the prior code!!

Updates #3137
Updates PR #3674
Requires PR #3694
Updates #3691
odeke-em added a commit to orijtech/opentelemetry-collector that referenced this pull request Aug 4, 2021
This change wires together the logic to convert
the pdata converters into a method ToMetricPdata
that appends converted metrics. The next change or two,
will then hook everything up directly and will
allow us to delete all the prior code!!

Updates #3137
Updates PR open-telemetry#3674
Requires PR open-telemetry#3694
Updates open-telemetry#3691
odeke-em added a commit to orijtech/opentelemetry-collector that referenced this pull request Aug 4, 2021
This change wires together the logic to convert
the pdata converters into a method ToMetricPdata
that appends converted metrics. The next change or two,
will then hook everything up directly and will
allow us to delete all the prior code!!

Updates #3137
Updates PR open-telemetry#3674
Requires PR open-telemetry#3694
Updates open-telemetry#3691
odeke-em added a commit to orijtech/opentelemetry-collector that referenced this pull request Aug 12, 2021
This change wires together the logic to convert
the pdata converters into a method ToMetricPdata
that appends converted metrics. The next change or two,
will then hook everything up directly and will
allow us to delete all the prior code!!

Updates #3137
Updates PR open-telemetry#3674
Requires PR open-telemetry#3694
Updates open-telemetry#3691
odeke-em added a commit to orijtech/opentelemetry-collector that referenced this pull request Aug 17, 2021
This change wires together the logic to convert
the pdata converters into a method ToMetricPdata
that appends converted metrics. The next change or two,
will then hook everything up directly and will
allow us to delete all the prior code!!

Updates #3137
Updates PR open-telemetry#3674
Requires PR open-telemetry#3694
Updates open-telemetry#3691
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.

6 participants