-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Remove loki dependency, copy files from logproto #16822
Conversation
Foresight Summary
View More Details⭕ build-and-test-windows workflow has finished in 4 seconds (38 minutes 28 seconds less than
|
Job | Failed Steps | Tests | |
---|---|---|---|
windows-unittest-matrix | - 🔗 | N/A | See Details |
windows-unittest | - 🔗 | N/A | See Details |
✅ check-links workflow has finished in 1 minute 11 seconds (1 minute 45 seconds less than main
branch avg.) and finished at 8th Dec, 2022.
Job | Failed Steps | Tests | |
---|---|---|---|
changed files | - 🔗 | N/A | See Details |
check-links | - 🔗 | N/A | See Details |
✅ tracegen workflow has finished in 1 minute 36 seconds (1 minute 48 seconds less than main
branch avg.) and finished at 8th Dec, 2022.
Job | Failed Steps | Tests | |
---|---|---|---|
build-dev | - 🔗 | N/A | See Details |
publish-latest | - 🔗 | N/A | See Details |
publish-stable | - 🔗 | N/A | See Details |
✅ changelog workflow has finished in 2 minutes 48 seconds (2 minutes 21 seconds less than main
branch avg.) and finished at 8th Dec, 2022.
Job | Failed Steps | Tests | |
---|---|---|---|
changelog | - 🔗 | N/A | See Details |
✅ prometheus-compliance-tests workflow has finished in 11 minutes 25 seconds (⚠️ 2 minutes 11 seconds more than main
branch avg.) and finished at 8th Dec, 2022.
Job | Failed Steps | Tests | |
---|---|---|---|
prometheus-compliance-tests | - 🔗 | ✅ 21 ❌ 0 ⏭ 0 🔗 | See Details |
build-and-test workflow has finished in 30 minutes 37 seconds (31 minutes 55 seconds less than main
branch avg.) and finished at 8th Dec, 2022.
Job | Failed Steps | Tests | |
---|---|---|---|
unittest-matrix (1.18, internal) | N/A | ✅ 592 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.19, internal) | N/A | ✅ 592 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.18, processor) | N/A | ✅ 1465 ❌ 0 ⏭ 0 🔗 | See Details |
correctness-traces | N/A | ✅ 17 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.18, extension) | N/A | ✅ 528 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.19, processor) | N/A | ✅ 1465 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.19, extension) | N/A | ✅ 528 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.19, receiver-0) | N/A | ✅ 2533 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.18, receiver-0) | N/A | ✅ 2533 ❌ 0 ⏭ 0 🔗 | See Details |
correctness-metrics | N/A | ✅ 2 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.19, exporter) | N/A | ✅ 2417 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.18, other) | N/A | ✅ 4205 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.19, receiver-1) | N/A | ✅ 1851 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.18, receiver-1) | N/A | ✅ 1851 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.18, exporter) | N/A | ✅ 2417 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.19, other) | N/A | ✅ 4205 ❌ 0 ⏭ 0 🔗 | See Details |
integration-tests | N/A | ✅ 57 ❌ 0 ⏭ 0 🔗 | See Details |
✅ load-tests workflow has finished in 17 minutes 57 seconds and finished at 8th Dec, 2022.
Job | Failed Steps | Tests | |
---|---|---|---|
loadtest (TestTraceAttributesProcessor) | - 🔗 | ✅ 3 ❌ 0 ⏭ 0 🔗 | See Details |
loadtest (TestIdleMode) | - 🔗 | ✅ 1 ❌ 0 ⏭ 0 🔗 | See Details |
loadtest (TestMetric10kDPS|TestMetricsFromFile) | - 🔗 | ✅ 6 ❌ 0 ⏭ 0 🔗 | See Details |
loadtest (TestTraceNoBackend10kSPS|TestTrace1kSPSWithAttrs) | - 🔗 | ✅ 8 ❌ 0 ⏭ 0 🔗 | See Details |
loadtest (TestTraceBallast1kSPSWithAttrs|TestTraceBallast1kSPSAddAttrs) | - 🔗 | ✅ 10 ❌ 0 ⏭ 0 🔗 | See Details |
loadtest (TestBallastMemory|TestLog10kDPS) | - 🔗 | ✅ 19 ❌ 0 ⏭ 0 🔗 | See Details |
setup-environment | - 🔗 | N/A | See Details |
loadtest (TestMetricResourceProcessor|TestTrace10kSPS) | - 🔗 | ✅ 12 ❌ 0 ⏭ 0 🔗 | See Details |
*You can configure Foresight comments in your organization settings page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems ok, so long at the code owners for the loki exporter are ok with this approach. Is the gopsutil downgrade needed in this pr?
That tag was removed from the gopsutil repo so either the downgrade is needed or an |
I pulled the downgrade into a separate pr: #16824 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very similar to what I have in a local WIP branch, with the addition that I have a readme in the proto directory outlining what changes were made, so that whenever it gets updated, we know how to re-do these changes. Given that this seems to be blocking the release, I'm giving my +1.
I was currently trying to backport the changes to the proto files and re-generating the Go files based on that so that we wouldn't make manual changes to the generated files, but that can wait.
45b4a2d
to
71a4f07
Compare
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
@bogdandrutu could you please clarify why this change was necessary?
Promtail receiver was excluded from the release and that unblocked the release. Can we revert this PR? and use logproto from loki directly instead of copying files? What cons do you see in this approach. |
Files copied from
github.com/grafana/loki/pkg/logproto
to remove unnecessary dependencies. Inlogproto.pb.go
I had to remove few typesQuery[Request|Response]
andSampleQuery[Request|Response]
and the gRPC service that uses them, because they depend on another loki packagestats
.Fixes #16821