OTLP over message systems #157

alexvanboxel · 2021-05-16T15:09:53Z

No description provided.

arminru · 2021-05-17T09:05:52Z

text/0000-otlp-over-messaging-systems.md

@@ -0,0 +1,110 @@
+# OTLP over messaging systems


Please make sure to rename your file to the PR # you've been assigned (0157).

tigrannajaryan

I am temporarily blocking this since I have several questions that are not answered by the OTEP and I think they should be answered before we commit to making this part of the Otel spec.

tigrannajaryan · 2021-05-17T20:02:43Z

text/0000-otlp-over-messaging-systems.md

+| `jaeger_proto` | X | - | - | the payload is deserialized to a single Jaeger proto `Span` |
+| `jaeger_json` | X | - | - | the payload is deserialized to a single Jaeger JSON Span using `jsonpb` |
+| `zipkin_proto` | X | - | - | the payload is deserialized into a list of Zipkin proto spans |
+| `zipkin_json` | X | - | - | the payload is deserialized into a list of Zipkin V2 JSON spans |
+| `zipkin_thrift` | X | - | - | the payload is deserialized into a list of Zipkin Thrift spans |


These are unrelated to OTLP. I think the shouldn't be in this OTEP.

I will remove it, but I'll leave it here to give other people a chance to comment.

tigrannajaryan · 2021-05-17T20:05:34Z

text/0000-otlp-over-messaging-systems.md

+
+Optionally an implementation could support `otlp_json` or other alternative
+encodings like jaeger or zipkin payloads. Alternative encodings should be configured
+by an `encoding` field in the configuration file.


What configuration file is this referring to? Does this assume that every implementation of OTLP over messaging system must have a configuration file? It seems like a stretch to me. Can it be a in-memory config option? Or an API parameter of a library that implements this OTEP?

tigrannajaryan · 2021-05-17T20:08:37Z

text/0000-otlp-over-messaging-systems.md

+For log data, implementors of a receiver are encouraged to also support
+receiving raw data from a topic. This enables scenarios were
+non-OpenTelemetry log producers can produce a stream of logs, either
+structured or unstructured, on that topic and the receiver wraps the log
+data in a configurable resource.


This seems unrelated to OTLP. Should it be in this OTEP?

Maybe not, it would be nice though that implements do this. But maybe it shouldn't be in the spec.

text/0000-otlp-over-messaging-systems.md

tigrannajaryan · 2021-05-17T20:16:38Z

text/0000-otlp-over-messaging-systems.md

+
+The `kafkareceiver` and `kafkaexporter` already implement this OTLP as
+described in the OpenTelemetry Collector. The description in th OTEP
+makes both of them compliant without modification. Although it


This is good. But let's also be aware this is not a must-have requirement. If we believe what Collector does is not the best we can do then we should be able to propose something different in this OTEP and fix the Collector (it is still in Beta and breaking changes are allowed).

tigrannajaryan · 2021-05-17T20:20:21Z

text/0000-otlp-over-messaging-systems.md

+spec, to leverage the conventions for attributes?
+
+No guaranty of order could be given, because not all systems support order.
+Is that a problem?


This should not be a problem since OTLP conceptually does not guarantee order of delivery in network protocol.

tigrannajaryan · 2021-05-17T20:25:38Z

text/0000-otlp-over-messaging-systems.md

+The default implementation must implement `otlp_proto`, meaning that the
+payload is Protobuf serialized from `ExportTraceServiceRequest` for traces,
+`ExportMetricsServiceRequest` for metrics, or `ExportLogsServiceRequest` for


One thing that is not clear to me is if we want to include the signal type in the serialized payload so that the recipient can tell what signal is. Since this OTEP does not define it it means the recipient that works with multiple signals needs another way to figure it out (often ends up using the channel name). I think this is an important capability that adds value and has no obvious downsides.

This is also an issue we want to solve for OTLP/File format: open-telemetry/opentelemetry-specification#1443 (comment)
It will be good to solve this here as well and do it in the same way we will do for OTLP/File so this likely requires to think through the OTLP/File design as well.

This doesn't help with the file format at all, but Kafka and Pubsub (maybe others) have key-value headers/attributes which are mentioned in "Open questions". The signal type and encoding (json or proto) could go in there

I've added a comment in the OTLP/File discussion about CloudEvents. Certainly, if you want to allow multiplexing logs, traces, and metrics over a single topic CloudEvents is a good fit and it would align with this OTLP as well as the spec defines bindings for different messaging systems:

Kafka

MQTT

NATS

Google Pubsub

@alexvanboxel I read the comment you left in the OTLP/File issue but I am a bit confused why it talks about the CloudEvents or Avro.

My comment here is about how an OTLP payload that is encoded as a Protobuf or JSON should be recorded in messaging systems such that the signal type information is included. I think we should keep it as simple as possible, new any additional concepts and technology introduced (CloudEvents or Avro) IMO is unnecessary complication.

If we want to record the type of the signal there are fairly ways to achieve it while staying with the existing set of technologies. For example we can add this message to OTLP Protobuf definitions:

// This may be also named simply Request or Payload or something like that message ExportServiceRequest { oneof request { ExportTraceServiceRequest traces = 1; ExportMetricsServiceRequest metrics = 2; ExportLogsServiceRequest logs = 3; } }

ExportServiceRequest can be then serialized and become the payload in the messaging system. Recipients can then deserialize and will be able to use the right signal in the "request" field.

@tigrannajaryan the consensus is that we want to signal the type of the message, so I'm already changing my application. Let me demonstrate what I mean by leveraging the CloudEvent spec (as it's a spec and not technology). In my Google Pubsub implementation, it describes what headers to use. So I could simple use:

func (receiver *pubsubReceiver) detectEncoding(attributes map[string]string) (Encoding, error) { ceType := attributes["ce-type"] ceContentType := attributes["ce-datacontenttype"] if strings.HasSuffix(ceContentType,"application/x-protobuf") { switch ceType { case "org.opentelemetry.otlp.traces.v1": return OtlpProtoTrace, nil case "org.opentelemetry.otlp.metrics.v1": return OtlpProtoMetric, nil case "org.opentelemetry.otlp.logs.v1": return OtlpProtoLog, nil } } else if strings.HasSuffix(ceContentType,"application/json") { switch ceType { case "org.opentelemetry.otlp.traces.v1": return OtlpJsonTrace, nil case "org.opentelemetry.otlp.metrics.v1": return OtlpJsonMetric, nil case "org.opentelemetry.otlp.logs.v1": return OtlpJsonLog, nil } } return Unknown, nil }

Granted, it's heavier than a oneOf, but it leaves the flexibility of even signaling OTLP/Json encoding.

text/0000-otlp-over-messaging-systems.md

aabmass · 2021-05-17T22:06:01Z

text/0000-otlp-over-messaging-systems.md

+The default implementation must implement `otlp_proto`, meaning that the
+payload is Protobuf serialized from `ExportTraceServiceRequest` for traces,
+`ExportMetricsServiceRequest` for metrics, or `ExportLogsServiceRequest` for


This doesn't help with the file format at all, but Kafka and Pubsub (maybe others) have key-value headers/attributes which are mentioned in "Open questions". The signal type and encoding (json or proto) could go in there

aabmass · 2021-05-17T22:08:26Z

text/0000-otlp-over-messaging-systems.md

+for reading raw log events avoids having an additional parallel
+implementation todo just that.
+
+The encoding field should be used to indicate that the data received is not the


What is meant by "field" here?

aabmass · 2021-05-17T22:11:51Z

text/0000-otlp-over-messaging-systems.md

+
+## Open questions
+
+This proposal doesn't take advantage of attributes that some systems


+1 I think it would be good to put encoding and signal type in there.

+1. I think perhaps having a standard convention for knowing what signal is coming in

(either force separate channels for each signal, OR a standard header of some sort to disambiguate) would be good. OTLP effectively has this in their GRPC definition via the separate services.

Well, gRPC can handle this over the same endpoint (a separate service is just a path in the HTTP/2 header).

I'm leaning toward the CloudEvents proposal, where you can have multiple (at least have the possibility to) types over one topic.

aabmass · 2021-05-17T22:19:59Z

text/0000-otlp-over-messaging-systems.md

@@ -0,0 +1,110 @@
+# OTLP over messaging systems
+
+Best practices for transporting OTLP data over a message system.


Do we have any performance best practices for message systems based on the existing Collector Kafka implementation? For example should you prefer to batch into many small messages or into fewer large messages?

I would not make it part of the spec, just like it is not part of the OTLP/gRPC spec.

jrcamp · 2021-05-17T23:08:12Z

text/0000-otlp-over-messaging-systems.md

+
+## Prior art and alternatives
+
+The `kafkareceiver` and `kafkaexporter` already implement this OTLP as


Note that I'm proposing some changes to these:

open-telemetry/opentelemetry-collector#3044
open-telemetry/opentelemetry-collector#3200

Still a WIP.

jrcamp · 2021-05-17T23:13:29Z

text/0000-otlp-over-messaging-systems.md

+| `jaeger_proto` | X | - | - | the payload is deserialized to a single Jaeger proto `Span` |
+| `jaeger_json` | X | - | - | the payload is deserialized to a single Jaeger JSON Span using `jsonpb` |
+| `zipkin_proto` | X | - | - | the payload is deserialized into a list of Zipkin proto spans |
+| `zipkin_json` | X | - | - | the payload is deserialized into a list of Zipkin V2 JSON spans |
+| `zipkin_thrift` | X | - | - | the payload is deserialized into a list of Zipkin Thrift spans |


I'm proposing changes to these in PR linked above. The types would be something like zipkinv1/thrift, zipkinv2/protobuf. Aside from the formatting it treats zipkinv1 and zipkinv2 as two separate protocols instead of putting them both behind zipkin.

I think I'm going to remove the other encoding out of the spec as @tigrannajaryan suggests

Co-authored-by: Tigran Najaryan <4194920+tigrannajaryan@users.noreply.github.com>

Co-authored-by: Aaron Abbott <aaronabbott@google.com>

tedsuo · 2023-07-24T16:45:23Z

@alexvanboxel we are marking OTEPs which are no longer being updated as stale. We will close this PR in one week. If you wish to resurrect this PR, that is fine.

alexvanboxel · 2023-07-25T07:16:45Z

I've neglected some of my OpenTelemetry work for the last few months. I will refocus some of my time back on open source. I will pick it back up, including the work in the collector.

alexvanboxel requested a review from a team May 16, 2021 15:09

alexvanboxel force-pushed the feature/message branch from cc98959 to a3508b4 Compare May 16, 2021 15:14

OTLP over messaging systems

78c3eb7

alexvanboxel force-pushed the feature/message branch from a3508b4 to 78c3eb7 Compare May 16, 2021 15:18

arminru reviewed May 17, 2021

View reviewed changes

tigrannajaryan requested changes May 17, 2021

View reviewed changes

aabmass reviewed May 17, 2021

View reviewed changes

jrcamp reviewed May 17, 2021

View reviewed changes

alexvanboxel and others added 3 commits May 18, 2021 07:56

Update text/0000-otlp-over-messaging-systems.md

dfd49e5

Co-authored-by: Tigran Najaryan <4194920+tigrannajaryan@users.noreply.github.com>

Update text/0000-otlp-over-messaging-systems.md

6cd071c

Co-authored-by: Aaron Abbott <aaronabbott@google.com>

Update text/0000-otlp-over-messaging-systems.md

57451c6

Co-authored-by: Aaron Abbott <aaronabbott@google.com>

This was referenced May 26, 2021

Add Google Pubsub exporter for OTLP messages - PR1 open-telemetry/opentelemetry-collector-contrib#3551

Merged

Add Google Pubsub receiver for OTLP messages - PR1 open-telemetry/opentelemetry-collector-contrib#3552

Merged

tedsuo added the triaged label Feb 6, 2023

tedsuo added the stale This issue or PR is stale and will be closed soon unless it is resurrected by the author. label Jul 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OTLP over message systems #157

OTLP over message systems #157

alexvanboxel commented May 16, 2021

arminru May 17, 2021

tigrannajaryan left a comment

tigrannajaryan May 17, 2021

alexvanboxel May 18, 2021

tigrannajaryan May 17, 2021

tigrannajaryan May 17, 2021

alexvanboxel May 18, 2021

tigrannajaryan May 17, 2021

tigrannajaryan May 17, 2021

tigrannajaryan May 17, 2021 •

edited

Loading

aabmass May 17, 2021

alexvanboxel May 18, 2021

tigrannajaryan May 19, 2021

alexvanboxel May 20, 2021 •

edited

Loading

aabmass May 17, 2021

aabmass May 17, 2021

aabmass May 17, 2021

jsuereth May 18, 2021

alexvanboxel May 18, 2021

aabmass May 17, 2021

alexvanboxel May 18, 2021

jrcamp May 17, 2021

jrcamp May 17, 2021 •

edited

Loading

alexvanboxel May 18, 2021

tedsuo commented Jul 24, 2023

alexvanboxel commented Jul 25, 2023


		## Open questions

		This proposal doesn't take advantage of attributes that some systems

		@@ -0,0 +1,110 @@
		# OTLP over messaging systems

		Best practices for transporting OTLP data over a message system.


		## Prior art and alternatives

		The `kafkareceiver` and `kafkaexporter` already implement this OTLP as

OTLP over message systems #157

Are you sure you want to change the base?

OTLP over message systems #157

Conversation

alexvanboxel commented May 16, 2021

Choose a reason for hiding this comment

tigrannajaryan 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

Choose a reason for hiding this comment

tigrannajaryan May 17, 2021 • edited Loading

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

alexvanboxel May 20, 2021 • edited Loading

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

jrcamp May 17, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tedsuo commented Jul 24, 2023

alexvanboxel commented Jul 25, 2023

tigrannajaryan May 17, 2021 •

edited

Loading

alexvanboxel May 20, 2021 •

edited

Loading

jrcamp May 17, 2021 •

edited

Loading