ingestion: native otlp ingestion support #10727

sandeepsukhani · 2023-09-27T11:27:55Z

What this PR does / why we need it:
Add support for natively supporting logs ingestion in OTLP format.
/otlp/v1/logs is the new endpoint where users can push logs in OTLP format. It accepts logs serialized in JSON or proto format. Since OTEL format is very different than what Loki storage model, here is how data in OTEL format will be mapped to Loki data model:

Index labels: The Resource Attributes map quite well to Index labels in Loki since both usually identify the source of the logs. The problem however is that Resource attributes in OTLP can have an unbounded number of values while Loki has a default limit of having up to 30 labels. Since Index labels in Loki can largely drive the kind of querying experience the users are going to have, we have chosen select attributes which would be picked as Index Labels. The ones that are not picked up as Index labels would be stored as Structured Metadata with each log entry.
Timestamp: LogRecord.TimeUnixNano
LogLine: LogRecord.Body holds the body of the log. However, since Loki only supports Log body in string format, we will stringify non-string values using AsString method from OTEL collector lib.
Structured Metadata: Anything which can’t be stored in Index labels and LogLine. Here is a non-exhaustive list of what will be stored in Structured Metadata to give a sense of what it will hold:
- Resource Attributes not stored as Index labels is replicated and stored with each log entry.
- Everything under InstrumentationScope is replicated and stored with each log entry.
- Everything under LogRecord except LogRecord.Body, LogRecord.TimeUnixNano and sometimes LogRecord.ObservedTimestamp.

NOTES:

Since Loki does not support . or any other special characters other than _ in label names, we replace all non-supported characters with _.
Since Loki only supports string in values of Index Labels and Structured Metadata, all the complex types are converted as follows:
- Map would be flattened into label keys using _ as separator, same as how we do it in json parser in LogQL.
- Everything else is stringified using AsString method from OTEL collector lib

Special notes for your reviewer:
I will open follow-up PRs for:

Documentation
Make blessed attributes list configurable per tenant.

Checklist

Tests updated
CHANGELOG.md updated
- If the change is worth mentioning in the release notes, add add-to-release-notes label

vlad-diachenko · 2023-10-03T16:53:14Z

pkg/loghttp/push/otlp.go

+
+		// copy blessed attributes to stream labels
+		for _, ba := range blessedAttributes {
+			normalizedBlessedAttribute := prometheustranslator.NormalizeLabel(ba)


we do not need to normalize it at runtime, we can generate normalizedBlessedAttributes on startup

…equest

…n tests

vlad-diachenko

Looks great! minor comments

vlad-diachenko · 2023-10-19T07:15:06Z

integration/client/client.go

+
+	buf, err := io.ReadAll(res.Body)
+	if err != nil {
+		return fmt.Errorf("reading request failed with status code %v: %w", res.StatusCode, err)


Suggested change

return fmt.Errorf("reading request failed with status code %v: %w", res.StatusCode, err)

return fmt.Errorf("reading response failed for status code %v: %w", res.StatusCode, err)

vlad-diachenko · 2023-10-19T07:16:57Z

integration/client/client.go

+		return fmt.Errorf("reading request failed with status code %v: %w", res.StatusCode, err)
+	}
+
+	return fmt.Errorf("request failed with status code %v: %w", res.StatusCode, errors.New(string(buf)))


what is the difference between these two formats? I believe we can just skip creating error from the string and use the string directly in the error message

Suggested change

return fmt.Errorf("request failed with status code %v: %w", res.StatusCode, errors.New(string(buf)))

return fmt.Errorf("request failed with status code %v: %s", res.StatusCode, string(buf))

%w is used for wrapping the errors, which the consumer can then unwrap. I just copied the code from the other push function. Wrapping the error here doesn't make sense, so I will change it.

vlad-diachenko · 2023-10-19T07:21:22Z

integration/loki_micro_services_test.go


 				// ingest logs to the current period
-				require.NoError(t, cliDistributor.PushLogLineWithStructuredMetadata("lineC", map[string]string{"traceID": "789"}, map[string]string{"job": "fake"}))
-				require.NoError(t, cliDistributor.PushLogLineWithStructuredMetadata("lineD", map[string]string{"traceID": "123"}, map[string]string{"job": "fake"}))
+				require.NoError(t, cliDistributor.PushLogLine("lineC", now, nil, map[string]string{"job": "fake"}))


why did we remove structured metadata from these log lines?
map[string]string{"traceID": "123"}

good catch, it was a mistake. Fixed it.

vlad-diachenko · 2023-10-19T07:27:14Z

integration/loki_micro_services_test.go

+		for i, stream := range resp.Data.Stream {
+			switch i {
+			case 0:
+				require.Len(t, stream.Values, 2)
+				require.Equal(t, "lineD", stream.Values[0][1])
+				require.Equal(t, "lineB", stream.Values[1][1])
+				require.Equal(t, map[string]string{
+					"service_name": "varlog",
+				}, stream.Stream)
+			case 1:
+				require.Len(t, stream.Values, 1)
+				require.Equal(t, "lineA", stream.Values[0][1])
+				require.Equal(t, map[string]string{
+					"service_name": "varlog",
+					"trace_id":     "1",
+					"user_id":      "2",
+				}, stream.Stream)
+			case 2:
+				require.Len(t, stream.Values, 1)
+				require.Equal(t, "lineC", stream.Values[0][1])
+				require.Equal(t, map[string]string{
+					"service_name": "varlog",
+					"order_ids":    "[5,6]",
+				}, stream.Stream)
+			default:
+				t.Errorf("unexpected case %d", i)
+			}
+		}


these assertions do not make sure that there are exactly 3 log lines in the response. If the response has only 1 line it will assert the first line and will mark the test as passed

salvacorts

Awesome work Sandeep! I left some comments.

pkg/loghttp/push/otlp.go

salvacorts · 2023-10-19T08:16:20Z

pkg/loghttp/push/push.go

+	return req, nil
+}
+
+func ParseHTTPRequest(userID string, r *http.Request, tenantsRetention TenantsRetention) (*logproto.PushRequest, *Stats, error) {


nit: Since OTEL can send HTTP requests as well, I'd rename this to ParseLokiRequest

salvacorts · 2023-10-19T08:19:51Z

pkg/loghttp/push/otlp.go

+	}
+}
+
+type Stats struct {


nit: since these structs are also used in push.go, I think moving them there makes more sense. Alternatively, we may create a new stats.go file

salvacorts · 2023-10-19T08:33:51Z

pkg/loghttp/push/otlp.go

+
+		// copy blessed attributes to stream labels
+		streamLabels := make(model.LabelSet, len(blessedAttributesNormalized))
+		for _, ba := range blessedAttributesNormalized {


I think here we should take the tenant's max_label_names_per_series into account and put the ones not fitting into structured metadata. Otherwise, we may add more labels than we allow and discard them later on.

The blessed attributes should be an ordered list by preference.

We are not going to enforce max_label_names_per_series limit here. The limits would be enforced later at the common code path which processed the parsed push requests. It would be the user's responsibility to set appropriate blessed attributes(we are going to make it configurable) to avoid rejecting data when the label count goes beyond the limits.

salvacorts · 2023-10-19T08:45:42Z

pkg/loghttp/push/otlp.go

+			}
+
+			// use fields and attributes from scope as structured metadata
+			scopeAttributesAsStructuredMetadata := attributesToLabels(scope.Attributes(), "")


This will allocate a push.LabelsAdapter of size scope.Attributes().Len() and then we will likely append to it up to 3 more items. I think we can an argument to attributesToLabels with the capacity we want to allocate for the returned labels adapter. Same applies to how we use it in otlpLogToPushEntry.

there is no way for the caller to know how big the slice has to be since the flattening of attributes could result in more elements when there are nested attributes. Also, passing that extra argument complicates the code. I would love to keep it simple and optimize it later if it happens to cause any with real usage.

salvacorts · 2023-10-19T08:49:39Z

pkg/loghttp/push/otlp.go

+				log := logs.At(k)
+
+				entry := otlpLogToPushEntry(log)
+				entry.StructuredMetadata = append(entry.StructuredMetadata, resourceAttributesAsStructuredMetadata...)


Here we'll potentially make two more allocations. I think we should do sth like:

entry := otlpLogToPushEntry(log) structuredMetadata := make(push.LabelsAdapter, 0, len(entry.StructuredMetadata)+len(scopeAttributesAsStructuredMetadata)+len(resourceAttributesAsStructuredMetadata)) structuredMetadata = append(structuredMetadata, entry.StructuredMetadata...) structuredMetadata = append(structuredMetadata, resourceAttributesAsStructuredMetadata...) structuredMetadata = append(structuredMetadata, scopeAttributesAsStructuredMetadata...) entry.StructuredMetadata = structuredMetadata

salvacorts · 2023-10-19T09:01:27Z

pkg/loghttp/push/otlp_test.go

+					},
+					{
+						Name:  "trace_id",
+						Value: "12345678123456781234567812345678",


It's a bit difficult to see how the traceID set in the payload maps to this output. Maybe we can have the traceID as a variable and call traceID.String() here?

I am not sure if that would be an ideal test. I actually copied the test case from otel repo.

salvacorts · 2023-10-19T09:02:29Z

pkg/loghttp/push/otlp.go

+	if traceID := log.TraceID(); !traceID.IsEmpty() {
+		structuredMetadata = append(structuredMetadata, push.LabelAdapter{
+			Name:  "trace_id",
+			Value: hex.EncodeToString(traceID[:]),


We can call traceID.String() here

It is not ideal to call String() as recommended by otel lib.

salvacorts · 2023-10-19T09:02:37Z

pkg/loghttp/push/otlp.go

+	if spanID := log.SpanID(); !spanID.IsEmpty() {
+		structuredMetadata = append(structuredMetadata, push.LabelAdapter{
+			Name:  "span_id",
+			Value: hex.EncodeToString(spanID[:]),


We can call spanID.String() here

salvacorts · 2023-10-19T09:03:42Z

pkg/loki/modules.go

@@ -321,19 +321,25 @@ func (t *Loki) initDistributor() (services.Service, error) {
 		tenant.WithDefaultResolver(tenant.NewMultiResolver())
 	}

-	pushHandler := middleware.Merge(
+	httpPushHandler := middleware.Merge(


Same comment wrt naming. I'd change it for lokiPushHandler

In fact provided they will likely always have the same middlewares I'd do:

httpPushHandlerMiddleware := middleware.Merge( serverutil.RecoveryHTTPMiddleware, t.HTTPAuthMiddleware, ) lokiPushHandler := httpPushHandlerMiddleware.Wrap(http.HandlerFunc(t.distributor.PushHandler)) otlpPushHandler := httpPushHandlerMiddleware.Wrap(http.HandlerFunc(t.distributor.OTLPPushHandler))

…loki (#11026) **What this PR does / why we need it**: In PR #10727, we added support for ingesting logs to loki in OTLP format. This PR adds the documentation on how to configure otel collector and how the data is mapped from OTLP format to Loki format. **Checklist** - [x] Documentation added --------- Co-authored-by: J Stickler <julie.stickler@grafana.com>

**What this PR does / why we need it**: Add support for natively supporting logs ingestion in OTLP format. `/otlp/v1/logs` is the new endpoint where users can push logs in OTLP format. It accepts logs serialized in JSON or proto format. Since OTEL format is very different than what Loki storage model, here is how data in OTEL format will be mapped to Loki data model: * Index labels: The Resource Attributes map quite well to Index labels in Loki since both usually identify the source of the logs. The problem however is that Resource attributes in OTLP can have an unbounded number of values while Loki has a default limit of having up to 30 labels. Since Index labels in Loki can largely drive the kind of querying experience the users are going to have, we have chosen select attributes which would be picked as Index Labels. The ones that are not picked up as Index labels would be stored as Structured Metadata with each log entry. * Timestamp: LogRecord.TimeUnixNano * LogLine: LogRecord.Body holds the body of the log. However, since Loki only supports Log body in string format, we will stringify non-string values using [AsString method from OTEL collector lib](https://github.com/open-telemetry/opentelemetry-collector/blob/ab3d6c5b64701e690aaa340b0a63f443ff22c1f0/pdata/pcommon/value.go#L353). * Structured Metadata: Anything which can’t be stored in Index labels and LogLine. Here is a non-exhaustive list of what will be stored in Structured Metadata to give a sense of what it will hold: * Resource Attributes not stored as Index labels is replicated and stored with each log entry. * Everything under InstrumentationScope is replicated and stored with each log entry. * Everything under LogRecord except LogRecord.Body, LogRecord.TimeUnixNano and sometimes LogRecord.ObservedTimestamp. *NOTES*: * Since Loki does not support `.` or any other special characters other than `_` in label names, we replace all non-supported characters with `_`. * Since Loki only supports string in values of Index Labels and Structured Metadata, all the complex types are converted as follows: * Map would be flattened into label keys using `_` as separator, same as how we do it in [json parser in LogQL](https://grafana.com/docs/loki/latest/query/log_queries/#json). * Everything else is stringified using [AsString method from OTEL collector lib](https://github.com/open-telemetry/opentelemetry-collector/blob/ab3d6c5b64701e690aaa340b0a63f443ff22c1f0/pdata/pcommon/value.go#L353) **Special notes for your reviewer**: I will open follow-up PRs for: * Documentation * Make blessed attributes list configurable per tenant. **Checklist** - [x] Tests updated - [x] `CHANGELOG.md` updated - [ ] If the change is worth mentioning in the release notes, add `add-to-release-notes` label

…loki (grafana#11026) **What this PR does / why we need it**: In PR grafana#10727, we added support for ingesting logs to loki in OTLP format. This PR adds the documentation on how to configure otel collector and how the data is mapped from OTLP format to Loki format. **Checklist** - [x] Documentation added --------- Co-authored-by: J Stickler <julie.stickler@grafana.com>

first-cut changes to transform otlp push payload to loki push format

03b5289

pull-request-size bot added the size/XL label Sep 27, 2023

sandeepsukhani changed the title ~~first-cut changes to transform otlp push payload to loki push format~~ ingestion: native otlp ingestion support Sep 27, 2023

vlad-diachenko reviewed Oct 3, 2023

View reviewed changes

sandeepsukhani added 3 commits October 4, 2023 17:18

some more tests to conversion of otlp to loki push format

c8e83f5

wire up things to expose otlp push api

158fdac

pre-allocate slices and maps wherever possible

5b820e3

pull-request-size bot added size/XXL and removed size/XL labels Oct 6, 2023

sandeepsukhani added 7 commits October 10, 2023 14:07

fix endpoint

8890742

keep blessed attributes normalized instead of doing it in each push r…

0f77846

…equest

Merge branch 'main' into sandeepsukhani/otlp-ingestion-support

9abc2b2

Merge branch 'main' into otlp-ingestion-support

255f5b6

add integration test for ingesting logs in otlp format

8e96c9e

reset boltdb-shipper singleton instances while cleaning up integratio…

5b8b1ef

…n tests

fmt imports

0992f77

sandeepsukhani marked this pull request as ready for review October 18, 2023 12:22

sandeepsukhani requested a review from a team as a code owner October 18, 2023 12:22

vlad-diachenko approved these changes Oct 19, 2023

View reviewed changes

salvacorts reviewed Oct 19, 2023

View reviewed changes

sandeepsukhani added 3 commits October 20, 2023 15:52

changes suggested from PR review

fb09330

Merge branch 'main' into sandeepsukhani/otlp-ingestion-support

f0776a7

lint and fix a mistake in integration test

e32d857

salvacorts approved these changes Oct 23, 2023

View reviewed changes

sandeepsukhani added 2 commits October 23, 2023 14:30

changes suggested from PR review

6234403

add changelog

abb8eac

sandeepsukhani enabled auto-merge (squash) October 23, 2023 09:32

sandeepsukhani merged commit 6069df8 into grafana:main Oct 23, 2023
4 checks passed

sandeepsukhani mentioned this pull request Oct 25, 2023

add documentation on how to use otel collector for ingesting logs to loki #11026

Merged

1 task

wildum mentioned this pull request Apr 11, 2024

Sync native otlp ingestion with Promtail grafana/alloy#316

Open

Kielek mentioned this pull request Mar 13, 2024

Loki Exporter open-telemetry/opentelemetry-dotnet-contrib#1548

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ingestion: native otlp ingestion support #10727

ingestion: native otlp ingestion support #10727

sandeepsukhani commented Sep 27, 2023 •

edited

vlad-diachenko Oct 3, 2023

vlad-diachenko left a comment

vlad-diachenko Oct 19, 2023

vlad-diachenko Oct 19, 2023

sandeepsukhani Oct 20, 2023

vlad-diachenko Oct 19, 2023

sandeepsukhani Oct 20, 2023

vlad-diachenko Oct 19, 2023

salvacorts left a comment

salvacorts Oct 19, 2023

salvacorts Oct 19, 2023

salvacorts Oct 19, 2023 •

edited

salvacorts Oct 19, 2023

sandeepsukhani Oct 20, 2023

salvacorts Oct 19, 2023

sandeepsukhani Oct 20, 2023

salvacorts Oct 19, 2023

salvacorts Oct 19, 2023

sandeepsukhani Oct 20, 2023

salvacorts Oct 19, 2023

sandeepsukhani Oct 20, 2023

salvacorts Oct 19, 2023

sandeepsukhani Oct 20, 2023

salvacorts Oct 19, 2023

salvacorts Oct 19, 2023

	return fmt.Errorf("reading request failed with status code %v: %w", res.StatusCode, err)
	return fmt.Errorf("reading response failed for status code %v: %w", res.StatusCode, err)

	return fmt.Errorf("request failed with status code %v: %w", res.StatusCode, errors.New(string(buf)))
	return fmt.Errorf("request failed with status code %v: %s", res.StatusCode, string(buf))

ingestion: native otlp ingestion support #10727

ingestion: native otlp ingestion support #10727

Conversation

sandeepsukhani commented Sep 27, 2023 • edited

Choose a reason for hiding this comment

vlad-diachenko left a 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

salvacorts left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

salvacorts Oct 19, 2023 • edited

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

Choose a reason for hiding this comment

sandeepsukhani commented Sep 27, 2023 •

edited

salvacorts Oct 19, 2023 •

edited