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: roundtrip Prometheus->Pdata direct conversion without OpenCensus #3741

Conversation

odeke-em
Copy link
Member

Wire up and use the direct Prometheus->Pdata conversion end to end.
With this change the receiver will no longer need OpenCensus.

This change will involve more follow-ups that just migrate over the tests,
because we don't want a super bloated/massive PR.

Fixes #3691
Depends on PR #3694
Depends on PR #3695

@odeke-em odeke-em requested a review from a team July 30, 2021 01:34
@odeke-em odeke-em force-pushed the receiver-prometheus-wireUp-roundTrip branch 3 times, most recently from 73b5f3c to 6cbd772 Compare July 30, 2021 08:41
@odeke-em odeke-em force-pushed the receiver-prometheus-wireUp-roundTrip branch from 6cbd772 to d9343be Compare July 31, 2021 00:20
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jul 31, 2021

CLA Signed

The committers are authorized under a signed CLA.

@tigrannajaryan
Copy link
Member

Please rebase and fix conflicts.

@odeke-em odeke-em force-pushed the receiver-prometheus-wireUp-roundTrip branch 4 times, most recently from 59fbe48 to e4163c9 Compare August 5, 2021 08:19
@bogdandrutu
Copy link
Member

ok  	go.opentelemetry.io/collector/receiver/otlpreceiver/internal/trace	0.058s
panic: interface conversion: v1.isMetric_Data is *v1.Metric_Sum, not *v1.Metric_Summary

goroutine 156 [running]:
go.opentelemetry.io/collector/model/pdata.Metric.Summary(...)
	D:/a/opentelemetry-collector/opentelemetry-collector/model/pdata/metrics.go:207
go.opentelemetry.io/collector/receiver/prometheusreceiver/internal.adjustStartTimestampPdata(0x40790ccccccccccd, 0xc000929708)
	D:/a/opentelemetry-collector/opentelemetry-collector/receiver/prometheusreceiver/internal/otlp_transaction.go:206 +0x1f0
go.opentelemetry.io/collector/receiver/prometheusreceiver/internal.(*transactionPdata).Commit(0xc0006328c0, 0x66fecc, 0xed89d97fb)
	D:/a/opentelemetry-collector/opentelemetry-collector/receiver/prometheusreceiver/internal/otlp_transaction.go:178 +0x1c5
github.com/prometheus/prometheus/scrape.(*scrapeLoop).scrapeAndReport.func1(0xc00001dce8, 0xc00001dcf8, 0xc00077f440)
	C:/Users/runneradmin/go/pkg/mod/github.com/prometheus/prometheus@v1.8.2-0.20210621150501-ff58416a0b02/scrape/scrape.go:1195 +0x50
github.com/prometheus/prometheus/scrape.(*scrapeLoop).scrapeAndReport(0xc00077f440, 0x3b9aca00, 0x3b9aca00, 0x0, 0x0, 0x0, 0x66fecc, 0xed89d97fb, 0x400bca0, 0x0, ...)
	C:/Users/runneradmin/go/pkg/mod/github.com/prometheus/prometheus@v1.8.2-0.20210621150501-ff58416a0b02/scrape/scrape.go:1262 +0xb58
github.com/prometheus/prometheus/scrape.(*scrapeLoop).run(0xc00077f440, 0x3b9aca00, 0x3b9aca00, 0x0)
	C:/Users/runneradmin/go/pkg/mod/github.com/prometheus/prometheus@v1.8.2-0.20210621150501-ff58416a0b02/scrape/scrape.go:1148 +0x37e
created by github.com/prometheus/prometheus/scrape.(*scrapePool).sync
	C:/Users/runneradmin/go/pkg/mod/github.com/prometheus/prometheus@v1.8.2-0.20210621150501-ff58416a0b02/scrape/scrape.go:564 +0xa0e

@odeke-em
Copy link
Member Author

odeke-em commented Aug 5, 2021


ok  	go.opentelemetry.io/collector/receiver/otlpreceiver/internal/trace	0.058s

panic: interface conversion: v1.isMetric_Data is *v1.Metric_Sum, not *v1.Metric_Summary



goroutine 156 [running]:

go.opentelemetry.io/collector/model/pdata.Metric.Summary(...)

	D:/a/opentelemetry-collector/opentelemetry-collector/model/pdata/metrics.go:207

go.opentelemetry.io/collector/receiver/prometheusreceiver/internal.adjustStartTimestampPdata(0x40790ccccccccccd, 0xc000929708)

	D:/a/opentelemetry-collector/opentelemetry-collector/receiver/prometheusreceiver/internal/otlp_transaction.go:206 +0x1f0

go.opentelemetry.io/collector/receiver/prometheusreceiver/internal.(*transactionPdata).Commit(0xc0006328c0, 0x66fecc, 0xed89d97fb)

	D:/a/opentelemetry-collector/opentelemetry-collector/receiver/prometheusreceiver/internal/otlp_transaction.go:178 +0x1c5

github.com/prometheus/prometheus/scrape.(*scrapeLoop).scrapeAndReport.func1(0xc00001dce8, 0xc00001dcf8, 0xc00077f440)

	C:/Users/runneradmin/go/pkg/mod/github.com/prometheus/prometheus@v1.8.2-0.20210621150501-ff58416a0b02/scrape/scrape.go:1195 +0x50

github.com/prometheus/prometheus/scrape.(*scrapeLoop).scrapeAndReport(0xc00077f440, 0x3b9aca00, 0x3b9aca00, 0x0, 0x0, 0x0, 0x66fecc, 0xed89d97fb, 0x400bca0, 0x0, ...)

	C:/Users/runneradmin/go/pkg/mod/github.com/prometheus/prometheus@v1.8.2-0.20210621150501-ff58416a0b02/scrape/scrape.go:1262 +0xb58

github.com/prometheus/prometheus/scrape.(*scrapeLoop).run(0xc00077f440, 0x3b9aca00, 0x3b9aca00, 0x0)

	C:/Users/runneradmin/go/pkg/mod/github.com/prometheus/prometheus@v1.8.2-0.20210621150501-ff58416a0b02/scrape/scrape.go:1148 +0x37e

created by github.com/prometheus/prometheus/scrape.(*scrapePool).sync

	C:/Users/runneradmin/go/pkg/mod/github.com/prometheus/prometheus@v1.8.2-0.20210621150501-ff58416a0b02/scrape/scrape.go:564 +0xa0e

Thanks @bogdandrutu! Indeed, I should perhaps mark this PR as a draft because I have to translate the entire test suite, it's basically consumed most of my time for the past week. I'll actually mail small parts of it progressively.

@odeke-em odeke-em marked this pull request as draft August 5, 2021 20:10
@odeke-em odeke-em force-pushed the receiver-prometheus-wireUp-roundTrip branch from e4163c9 to 8ac86ff Compare August 12, 2021 01:22
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
Wire up and use the direct Prometheus->Pdata conversion end to end.
With this change the receiver will no longer need OpenCensus.

This change will involve more follow-ups that just migrate over the tests,
because we don't want a super bloated/massive PR.

Fixes open-telemetry#3691
Depends on PR open-telemetry#3694
Depends on PR open-telemetry#3695
@odeke-em odeke-em force-pushed the receiver-prometheus-wireUp-roundTrip branch from 0a0d196 to 34e6396 Compare August 17, 2021 00:18
@odeke-em odeke-em force-pushed the receiver-prometheus-wireUp-roundTrip branch from f30e2ec to 3cc2119 Compare August 17, 2021 01:54
@odeke-em odeke-em force-pushed the receiver-prometheus-wireUp-roundTrip branch from 621a931 to 9c7274b Compare August 17, 2021 04:28
@github-actions
Copy link
Contributor

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

@bogdandrutu
Copy link
Member

@odeke-em unfortunately this needs to be moved to contrib. Sorry and thanks :)

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

Successfully merging this pull request may close these issues.

receiver/prometheus/internal: use actual startTimeMs of interval for toDoubleValue
3 participants