[receiver/loki] Add logic implementation #19217

mar4uk · 2023-03-06T17:23:47Z

Description:
The first PR with the skeleton is here #19399
This PR is the second one, with the concrete implementation of the component
Link to tracking Issue:
18635

Testing: Added unit tests

runforesight · 2023-03-06T17:25:01Z

Foresight Summary

Major Impacts

build-and-test-windows duration(13 seconds) has decreased 31 minutes 27 seconds compared to main branch avg(31 minutes 40 seconds).

View More Details

⭕ build-and-test-windows workflow has finished in 13 seconds (31 minutes 27 seconds less than `main` branch avg.) and finished at 5th Apr, 2023.

Job	Failed Steps	Tests
windows-unittest-matrix	- 🔗	N/A	See Details
windows-unittest	- 🔗	N/A	See Details

✅ check-links workflow has finished in 48 seconds and finished at 5th Apr, 2023.

Job	Failed Steps	Tests
changed files	- 🔗	N/A	See Details
check-links	- 🔗	N/A	See Details

✅ telemetrygen workflow has finished in 1 minute 7 seconds and finished at 5th Apr, 2023.

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 3 minutes 6 seconds (⚠️ 51 seconds more than `main` branch avg.) and finished at 5th Apr, 2023.

Job	Failed Steps	Tests
changelog	- 🔗	N/A	See Details

✅ build-and-test workflow has finished in 42 minutes 43 seconds and finished at 5th Apr, 2023.

Job	Failed Steps	Tests
unittest-matrix (1.19, connector)	- 🔗	✅ 126 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.20, connector)	- 🔗	✅ 126 ❌ 0 ⏭ 0 🔗	See Details
correctness-metrics	- 🔗	✅ 2 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.20, internal)	- 🔗	✅ 583 ❌ 0 ⏭ 0 🔗	See Details
correctness-traces	- 🔗	✅ 17 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, internal)	- 🔗	✅ 583 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.20, extension)	- 🔗	✅ 544 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, processor)	- 🔗	✅ 1557 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, extension)	- 🔗	✅ 544 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.20, processor)	- 🔗	✅ 1557 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, receiver-0)	- 🔗	✅ 2616 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.20, receiver-0)	- 🔗	✅ 2616 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.20, receiver-1)	- 🔗	✅ 1969 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, exporter)	- 🔗	✅ 2510 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.20, exporter)	- 🔗	✅ 2510 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, receiver-1)	- 🔗	✅ 1969 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.20, other)	- 🔗	✅ 4765 ❌ 0 ⏭ 0 🔗	See Details
integration-tests	- 🔗	✅ 56 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, other)	- 🔗	✅ 4765 ❌ 0 ⏭ 0 🔗	See Details
setup-environment	- 🔗	N/A	See Details
check-codeowners	- 🔗	N/A	See Details
build-examples	- 🔗	N/A	See Details
checks	- 🔗	N/A	See Details
check-collector-module-version	- 🔗	N/A	See Details
lint-matrix (receiver-0)	- 🔗	N/A	See Details
lint-matrix (receiver-1)	- 🔗	N/A	See Details
lint-matrix (processor)	- 🔗	N/A	See Details
lint-matrix (extension)	- 🔗	N/A	See Details
lint-matrix (exporter)	- 🔗	N/A	See Details
lint-matrix (connector)	- 🔗	N/A	See Details
lint-matrix (internal)	- 🔗	N/A	See Details
lint-matrix (other)	- 🔗	N/A	See Details
lint	- 🔗	N/A	See Details
unittest (1.20)	- 🔗	N/A	See Details
unittest (1.19)	- 🔗	N/A	See Details
cross-compile (darwin, amd64)	- 🔗	N/A	See Details
cross-compile (darwin, arm64)	- 🔗	N/A	See Details
cross-compile (linux, 386)	- 🔗	N/A	See Details
cross-compile (linux, amd64)	- 🔗	N/A	See Details
cross-compile (linux, arm)	- 🔗	N/A	See Details
cross-compile (linux, arm64)	- 🔗	N/A	See Details
cross-compile (linux, ppc64le)	- 🔗	N/A	See Details
cross-compile (windows, 386)	- 🔗	N/A	See Details
cross-compile (windows, amd64)	- 🔗	N/A	See Details
build-package (rpm)	- 🔗	N/A	See Details
build-package (deb)	- 🔗	N/A	See Details
windows-msi	- 🔗	N/A	See Details
publish-check	- 🔗	N/A	See Details
publish-stable	- 🔗	N/A	See Details
publish-dev	- 🔗	N/A	See Details
rotate-milestone	- 🔗	N/A	See Details

✅ prometheus-compliance-tests workflow has finished in 10 minutes 32 seconds (⚠️ 4 minutes 12 seconds more than `main` branch avg.) and finished at 5th Apr, 2023.

Job	Failed Steps	Tests
prometheus-compliance-tests	- 🔗	✅ 21 ❌ 0 ⏭ 0 🔗	See Details

✅ load-tests workflow has finished in 15 minutes 42 seconds (⚠️ 5 minutes 23 seconds more than `main` branch avg.) and finished at 5th Apr, 2023.

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 (TestMetricResourceProcessor\|TestTrace10kSPS)	- 🔗	✅ 12 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestTraceBallast1kSPSWithAttrs\|TestTraceBallast1kSPSAddAttrs)	- 🔗	✅ 10 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestBallastMemory\|TestLog10kDPS)	- 🔗	✅ 18 ❌ 0 ⏭ 0 🔗	See Details
setup-environment	- 🔗	N/A	See Details

✅ e2e-tests workflow has finished in 17 minutes 47 seconds (⚠️ 3 minutes 42 seconds more than `main` branch avg.) and finished at 5th Apr, 2023.

Job	Failed Steps	Tests
kubernetes-test (v1.26.0)	- 🔗	N/A	See Details
kubernetes-test (v1.25.3)	- 🔗	N/A	See Details
kubernetes-test (v1.24.7)	- 🔗	N/A	See Details
kubernetes-test (v1.23.13)	- 🔗	N/A	See Details

🔎 See details on Foresight

^{*You can configure Foresight comments in your organization settings page.}

mar4uk · 2023-03-23T08:07:25Z

@kovrus this PR is ready for review

pkg/translator/loki/loki_to_otlp.go

receiver/lokireceiver/internal/util.go

jpkrohling · 2023-03-27T20:01:05Z

receiver/lokireceiver/internal/util.go

+	if len(buffer.Bytes()) > maxSize {
+		return nil, fmt.Errorf(messageSizeLargerErrFmt, len(buffer.Bytes()), maxSize)
+	}
+	switch compression {


Isn't the only code path leading here already within a similar switch statement? Perhaps just have the logic from RawSnappy here directly, and perhaps even remove the compression type if snappy is the only supported compression mechanism at this point?

Refactored methods, removed compressionType argument, because currently only Snappy is supported

jpkrohling · 2023-03-27T20:02:42Z

receiver/lokireceiver/loki.go

-	return r, err
+	if conf.HTTP != nil {
+		r.httpMux = http.NewServeMux()
+		r.httpMux.HandleFunc("/loki/api/v1/push", func(resp http.ResponseWriter, req *http.Request) {


One thing we are missing on the OTLP receiver is the ability to let users specify a path prefix, making it easier to place this code behind a reverse proxy. What do you think about doing the same here?

Do you mean we can add prefix as field in the config?

receivers: loki: protocols: http: prefix: "/proxy"

and use it in handler:
r.httpMux.HandleFunc(fmt.Sprintf("%s/loki/api/v1/push", r.conf.prefix),...
like this?

Yes. Perhaps "pathPrefix" would be a better name? Or perhaps even something else?

This can be done afterward. I think we can merge this as is.

jpkrohling · 2023-03-27T20:03:26Z

receiver/lokireceiver/loki.go

+	}
+
+	if r.conf.GRPC != nil {
+		r.settings.Logger.Info("Creating GRPC server")


There seems to be a similar entry already in startGRPCServer.

removed this line, thank you!

jpkrohling · 2023-03-27T20:06:28Z

receiver/lokireceiver/loki.go

+func (r *lokiReceiver) Push(ctx context.Context, pushRequest *push.PushRequest) (*push.PushResponse, error) {
+	logs, err := loki.PushRequestToLogs(pushRequest, r.conf.KeepTimestamp)
+	if err != nil {
+		r.settings.Logger.Warn(ErrAtLeastOneEntryFailedToProcess, zap.Error(err))


This looks like a bad payload, I think it would be better to perhaps count the number of errors instead of logging a warning on every such error. Perhaps add this to debug level instead, so that people can see what's wrong in non-prod environments, or temporarily?

I added the number of errors as well.
This error is logged once per push request, which shouldn't be much.
I think it is good to have this warning in production so users can identify what requests contain incorrect labels.

While I still think that this has the potential to flood the logs, as a bad payload is typically a sign of a wrongly configured client, I trust your judgment here.

jpkrohling · 2023-03-29T19:09:30Z

receiver/lokireceiver/loki.go

-	return r, err
+	if conf.HTTP != nil {
+		r.httpMux = http.NewServeMux()
+		r.httpMux.HandleFunc("/loki/api/v1/push", func(resp http.ResponseWriter, req *http.Request) {


This can be done afterward. I think we can merge this as is.

jpkrohling · 2023-03-29T19:12:01Z

receiver/lokireceiver/loki.go

+func (r *lokiReceiver) Push(ctx context.Context, pushRequest *push.PushRequest) (*push.PushResponse, error) {
+	logs, err := loki.PushRequestToLogs(pushRequest, r.conf.KeepTimestamp)
+	if err != nil {
+		r.settings.Logger.Warn(ErrAtLeastOneEntryFailedToProcess, zap.Error(err))


While I still think that this has the potential to flood the logs, as a bad payload is typically a sign of a wrongly configured client, I trust your judgment here.

mar4uk · 2023-04-05T09:36:22Z

@jpkrohling can we merge this? or shall I wait for more approves?

github-actions bot added cmd/configschema configschema command exporter/loki Loki Exporter pkg/translator/loki labels Mar 6, 2023

github-actions bot requested review from dmitryax, gouthamve, gramidt, jpkrohling, kovrus, mx-psi and pmcollins March 6, 2023 17:24

mar4uk force-pushed the loki-receiver branch 7 times, most recently from 4c24100 to a8b5649 Compare March 7, 2023 10:55

mar4uk marked this pull request as ready for review March 7, 2023 12:08

mar4uk requested a review from a team March 7, 2023 12:08

github-actions bot assigned djaglowski Mar 7, 2023

djaglowski removed request for jpkrohling, pmcollins, gramidt, mx-psi, dmitryax and gouthamve March 7, 2023 14:11

djaglowski assigned kovrus and unassigned djaglowski Mar 7, 2023

mar4uk force-pushed the loki-receiver branch from a8b5649 to a9f5e69 Compare March 21, 2023 15:01

mar4uk requested review from jpkrohling, mx-psi and dmitryax as code owners March 21, 2023 15:01

github-actions bot added the receiver/loki label Mar 21, 2023

github-actions bot requested review from gouthamve, gramidt and pmcollins March 21, 2023 15:01

mar4uk force-pushed the loki-receiver branch 4 times, most recently from db10104 to 0f79045 Compare March 22, 2023 16:35

mar4uk changed the title ~~[WIP] [receiver/loki] New component lokireceiver 2~~ [receiver/loki] Added logic implementation Mar 22, 2023

jpkrohling changed the title ~~[receiver/loki] Added logic implementation~~ [receiver/loki] Add logic implementation Mar 27, 2023

jpkrohling reviewed Mar 27, 2023

View reviewed changes

mar4uk force-pushed the loki-receiver branch 3 times, most recently from 7b04776 to b635c45 Compare March 29, 2023 09:59

jpkrohling approved these changes Mar 29, 2023

View reviewed changes

mar4uk force-pushed the loki-receiver branch from b635c45 to f47ab68 Compare March 30, 2023 13:52

kovrus approved these changes Mar 31, 2023

View reviewed changes

mar4uk force-pushed the loki-receiver branch from f47ab68 to 67a6b82 Compare April 4, 2023 08:39

mar4uk added 2 commits April 5, 2023 09:38

[receiver/loki] Added implementation

598dd80

address review comments

3eebc6a

mar4uk force-pushed the loki-receiver branch from 67a6b82 to 3eebc6a Compare April 5, 2023 08:39

jpkrohling merged commit 13ce234 into open-telemetry:main Apr 5, 2023

github-actions bot added this to the next release milestone Apr 5, 2023

mar4uk mentioned this pull request Apr 11, 2023

[receiver/loki] Enable component #20853

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[receiver/loki] Add logic implementation #19217

[receiver/loki] Add logic implementation #19217

mar4uk commented Mar 6, 2023 •

edited

Loading

runforesight bot commented Mar 6, 2023 •

edited

Loading

`build-and-test-windows` duration(13 seconds) has decreased 31 minutes 27 seconds compared to main branch avg(31 minutes 40 seconds).

⭕ build-and-test-windows workflow has finished in 13 seconds (31 minutes 27 seconds less than `main` branch avg.) and finished at 5th Apr, 2023.

✅ check-links workflow has finished in 48 seconds and finished at 5th Apr, 2023.

✅ telemetrygen workflow has finished in 1 minute 7 seconds and finished at 5th Apr, 2023.

✅ changelog workflow has finished in 3 minutes 6 seconds (⚠️ 51 seconds more than `main` branch avg.) and finished at 5th Apr, 2023.

✅ build-and-test workflow has finished in 42 minutes 43 seconds and finished at 5th Apr, 2023.

✅ prometheus-compliance-tests workflow has finished in 10 minutes 32 seconds (⚠️ 4 minutes 12 seconds more than `main` branch avg.) and finished at 5th Apr, 2023.

✅ load-tests workflow has finished in 15 minutes 42 seconds (⚠️ 5 minutes 23 seconds more than `main` branch avg.) and finished at 5th Apr, 2023.

✅ e2e-tests workflow has finished in 17 minutes 47 seconds (⚠️ 3 minutes 42 seconds more than `main` branch avg.) and finished at 5th Apr, 2023.

mar4uk commented Mar 23, 2023

jpkrohling Mar 27, 2023

mar4uk Mar 28, 2023

jpkrohling Mar 27, 2023

mar4uk Mar 28, 2023

jpkrohling Mar 29, 2023

jpkrohling Mar 29, 2023

jpkrohling Mar 27, 2023

mar4uk Mar 28, 2023

jpkrohling Mar 27, 2023

mar4uk Mar 28, 2023

jpkrohling Mar 29, 2023

jpkrohling Mar 29, 2023

jpkrohling Mar 29, 2023

mar4uk commented Apr 5, 2023

[receiver/loki] Add logic implementation #19217

[receiver/loki] Add logic implementation #19217

Conversation

mar4uk commented Mar 6, 2023 • edited Loading

runforesight bot commented Mar 6, 2023 • edited Loading

Foresight Summary

build-and-test-windows duration(13 seconds) has decreased 31 minutes 27 seconds compared to main branch avg(31 minutes 40 seconds).

⭕ build-and-test-windows workflow has finished in 13 seconds (31 minutes 27 seconds less than main branch avg.) and finished at 5th Apr, 2023.

✅ check-links workflow has finished in 48 seconds and finished at 5th Apr, 2023.

✅ telemetrygen workflow has finished in 1 minute 7 seconds and finished at 5th Apr, 2023.

✅ changelog workflow has finished in 3 minutes 6 seconds (⚠️ 51 seconds more than main branch avg.) and finished at 5th Apr, 2023.

✅ build-and-test workflow has finished in 42 minutes 43 seconds and finished at 5th Apr, 2023.

✅ prometheus-compliance-tests workflow has finished in 10 minutes 32 seconds (⚠️ 4 minutes 12 seconds more than main branch avg.) and finished at 5th Apr, 2023.

✅ load-tests workflow has finished in 15 minutes 42 seconds (⚠️ 5 minutes 23 seconds more than main branch avg.) and finished at 5th Apr, 2023.

✅ e2e-tests workflow has finished in 17 minutes 47 seconds (⚠️ 3 minutes 42 seconds more than main branch avg.) and finished at 5th Apr, 2023.

mar4uk commented Mar 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mar4uk commented Apr 5, 2023

mar4uk commented Mar 6, 2023 •

edited

Loading

runforesight bot commented Mar 6, 2023 •

edited

Loading

`build-and-test-windows` duration(13 seconds) has decreased 31 minutes 27 seconds compared to main branch avg(31 minutes 40 seconds).

⭕ build-and-test-windows workflow has finished in 13 seconds (31 minutes 27 seconds less than `main` branch avg.) and finished at 5th Apr, 2023.

✅ changelog workflow has finished in 3 minutes 6 seconds (⚠️ 51 seconds more than `main` branch avg.) and finished at 5th Apr, 2023.

✅ prometheus-compliance-tests workflow has finished in 10 minutes 32 seconds (⚠️ 4 minutes 12 seconds more than `main` branch avg.) and finished at 5th Apr, 2023.

✅ load-tests workflow has finished in 15 minutes 42 seconds (⚠️ 5 minutes 23 seconds more than `main` branch avg.) and finished at 5th Apr, 2023.

✅ e2e-tests workflow has finished in 17 minutes 47 seconds (⚠️ 3 minutes 42 seconds more than `main` branch avg.) and finished at 5th Apr, 2023.