From c55650e0e32071fdc096339b4941722a8b64f1fd Mon Sep 17 00:00:00 2001 From: Jonas Kunz Date: Thu, 25 Jul 2024 18:32:47 +0200 Subject: [PATCH 1/3] Add ability for SpanProcessor to mutate spans on end (#4024) Fixes #1089. In addition to the comments on the issue, this was discussed in the spec SIG Meeting on 2024/23/04: * The filtering use-case explained in [this comment](https://github.com/open-telemetry/opentelemetry-specification/issues/1089#issuecomment-2061289086) should rather be solved by the upcoming samplerV2 instead of `SpanProcessor`s due to better conceptual fit * The buffering use-case also explained in [this comment](https://github.com/open-telemetry/opentelemetry-specification/issues/1089#issuecomment-2061289086) seems to be not relevant enough to influence the design decision * Apparently there was a discussion around building the `SpanProcessor`s in a chaining fashion during the initial SDK spec design and it was actively decided against it. However, no one could recall the reason why. --- CHANGELOG.md | 2 ++ spec-compliance-matrix.md | 1 + specification/trace/sdk.md | 34 +++++++++++++++++++++++++++++++++- 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea401b9dbc4..5c1d32f9e72 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ release. - Remove restriction that sampler description is immutable. ([#4137](https://github.com/open-telemetry/opentelemetry-specification/pull/4137)) +- Add in-development `OnEnding` callback to SDK `SpanProcessor` interface. + ([#4024](https://github.com/open-telemetry/opentelemetry-specification/pull/4024)) ### Metrics diff --git a/spec-compliance-matrix.md b/spec-compliance-matrix.md index b0d9f5c07ef..374e1e2b340 100644 --- a/spec-compliance-matrix.md +++ b/spec-compliance-matrix.md @@ -55,6 +55,7 @@ formats is required. Implementing more than one format is optional. | events collection size limit | | + | + | + | + | + | + | + | + | - | - | + | | attribute collection size limit | | + | + | + | + | + | + | + | + | - | - | + | | links collection size limit | | + | + | + | + | + | + | + | + | - | - | + | +| [SpanProcessor.OnEnding](specification/trace/sdk.md#onending) | X | - | - | - | - | - | - | - | - | - | - | - | | [Span attributes](specification/trace/api.md#set-attributes) | Optional | Go | Java | JS | Python | Ruby | Erlang | PHP | Rust | C++ | .NET | Swift | | SetAttribute | | + | + | + | + | + | + | + | + | + | + | + | | Set order preserved | X | + | - | + | + | + | + | + | + | + | + | + | diff --git a/specification/trace/sdk.md b/specification/trace/sdk.md index 622a1e1452b..82bec706359 100644 --- a/specification/trace/sdk.md +++ b/specification/trace/sdk.md @@ -38,6 +38,7 @@ linkTitle: SDK - [Span processor](#span-processor) * [Interface definition](#interface-definition) + [OnStart](#onstart) + + [OnEnding](#onending) + [OnEnd(Span)](#onendspan) + [Shutdown()](#shutdown) + [ForceFlush()](#forceflush) @@ -571,11 +572,23 @@ in the SDK: ### Interface definition +The `SpanProcessor` interface MUST declare the following methods: + +* [OnStart](#onstart) +* [OnEnd](#onendspan) +* [Shutdown](#shutdown-1) +* [ForceFlush](#forceflush-1) + +The `SpanProcessor` interface SHOULD declare the following methods: + +* [OnEnding](#onending) method. + #### OnStart `OnStart` is called when a span is started. This method is called synchronously on the thread that started the span, therefore it should not block or throw -exceptions. +exceptions. If multiple `SpanProcessors` are registered, their `OnStart` callbacks +are invoked in the order they have been registered. **Parameters:** @@ -590,6 +603,25 @@ exceptions. **Returns:** `Void` +#### OnEnding + +**Status**: [Development](../document-status.md) + +`OnEnding` is called during the span `End()` operation, after the end timestamp has been set. The Span object is still mutable (i.e., `SetAttribute`, `AddLink`, `AddEvent` can be called) while `OnEnding` is called. +This method MUST be called synchronously within the [`Span.End()` API](api.md#end), +therefore it should not block or throw an exception. +If multiple `SpanProcessors` are registered, their `OnEnding` callbacks +are invoked in the order they have been registered. +The SDK MUST guarantee that the span can no longer be modified by any other thread +before invoking `OnEnding` of the first `SpanProcessor`. From that point on, modifications +are only allowed synchronously from within the invoked `OnEnding` callbacks. All registered SpanProcessor `OnEnding` callbacks are executed before any SpanProcessor's `OnEnd` callback is invoked. + +**Parameters:** + +* `span` - a [read/write span object](#additional-span-interfaces) for the span which is about to be ended. + +**Returns:** `Void` + #### OnEnd(Span) `OnEnd` is called after a span is ended (i.e., the end timestamp is already set). From 1c7eb216b0c166c749101b2d6449cccbcb7a9a07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 26 Jul 2024 15:52:05 +0200 Subject: [PATCH 2/3] Allow cloning of ReadWriteLogRecord (#4090) Follows https://github.com/open-telemetry/opentelemetry-specification/pull/4062 ## Why 1. There is a need to make a deep copy of a `ReadWriteRecord` in order to implement an experimental isolating processor outside of the SDK (as it is experimental, we prefer to land it to contrib repository first). 2. Allow fine-grained control for the users to make a deep copy only when necessary. This allows users to have more control on the processing e.g. pass a clone to asynchronous processor to avoid race conditions or make performance improvements. --- CHANGELOG.md | 3 +++ specification/logs/sdk.md | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c1d32f9e72..2e71b526382 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,9 @@ release. ### Logs +- The SDK MAY provide an operation that makes a deep clone of a `ReadWriteLogRecord`. + ([#4090](https://github.com/open-telemetry/opentelemetry-specification/pull/4090)) + ### Events ### Resource diff --git a/specification/logs/sdk.md b/specification/logs/sdk.md index be06ddc56b6..eadd5a597d2 100644 --- a/specification/logs/sdk.md +++ b/specification/logs/sdk.md @@ -235,6 +235,12 @@ the following information added to the [LogRecord](data-model.md#log-and-event-r * [`SpanId`](./data-model.md#field-spanid) * [`TraceFlags`](./data-model.md#field-traceflags) +The SDK MAY provide an operation that makes a deep clone of a `ReadWriteLogRecord`. +The operation can be used to implement the [isolating processor](#isolating-processor) +or by asynchronous processors (e.g. [Batching processor](#batching-processor)) +to avoid race conditions on the log record that is not required to be +concurrent safe. + ## LogRecord Limits `LogRecord` attributes MUST adhere to From 64e772d8578045b25a69298c7ace69318592dd57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Mon, 29 Jul 2024 15:57:15 +0200 Subject: [PATCH 3/3] Use Body instead of Payload in Events SDK (#4158) Leftover after https://github.com/open-telemetry/opentelemetry-specification/pull/4035 which changed `Payload` into `Body` in Events API --- specification/logs/event-sdk.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specification/logs/event-sdk.md b/specification/logs/event-sdk.md index 5a4769fd33c..1a9bc0ee829 100644 --- a/specification/logs/event-sdk.md +++ b/specification/logs/event-sdk.md @@ -87,7 +87,7 @@ to [emit a logRecord](./bridge-api.md#emit-a-logrecord) as follows: the `event.name` [Attribute](./data-model.md#field-attributes). If the `Attributes` provided by the user contain an `event.name` attribute the value provided in the `Name` takes precedence. -* If provided by the user, the `Payload` MUST be used to set +* If provided by the user, the `Body` MUST be used to set the [Body](./data-model.md#field-body). If not provided, `Body` MUST not be set. * If provided by the user, the `Timestamp` MUST be used to set