From 1e67214bae0c274f9f0cc1e394849ff3391df273 Mon Sep 17 00:00:00 2001 From: Joao Grassi Date: Fri, 22 Jul 2022 11:40:53 +0200 Subject: [PATCH 1/6] Make it possible to indicate partial success in an OTLP export response --- CHANGELOG.md | 2 ++ .../collector/logs/v1/logs_service.proto | 31 +++++++++++++++++++ .../metrics/v1/metrics_service.proto | 31 +++++++++++++++++++ .../collector/trace/v1/trace_service.proto | 31 +++++++++++++++++++ 4 files changed, 95 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c2bf9fb8..9419507c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,8 @@ Full list of differences found in [this compare](https://github.com/open-telemet ### Added * Introduce Scope Attributes. [#395](https://github.com/open-telemetry/opentelemetry-proto/pull/395) +* Introduce partial success fields in `ExportServiceResponse`. + [#?](https://github.com/open-telemetry/opentelemetry-proto/pull/?) ### Removed diff --git a/opentelemetry/proto/collector/logs/v1/logs_service.proto b/opentelemetry/proto/collector/logs/v1/logs_service.proto index cc1162fd8..b98613e1d 100644 --- a/opentelemetry/proto/collector/logs/v1/logs_service.proto +++ b/opentelemetry/proto/collector/logs/v1/logs_service.proto @@ -43,4 +43,35 @@ message ExportLogsServiceRequest { } message ExportLogsServiceResponse { + // The details of a partially successful export request. + // + // If the request is only partially accepted + // (i.e. when the server accepts only parts of the data and rejects the rest) + // the server MUST initialize the `partial_success` field and MUST + // set the `rejected_log_records` with the number of log records it rejected. + // + // The server MUST NOT set the `partial_success` field when the + // request was accepted in its entirety. + ExportLogsPartialSuccess partial_success = 1; +} + +message ExportLogsPartialSuccess { + // The number of rejected log records. + // + // If the request is only partially accepted, the server MUST set + // the `rejected_log_records` field. + // + // Given the server should only initialize the `partial_success` field + // when it rejects parts of the data, a `rejected_log_records` with + // value = '0' is considered an invalid result. Senders should + // interpret such responses from a server as if all log records were accepted. + int64 rejected_log_records = 1; + + // A developer-facing human-readable error message in English. It should + // explain why the server rejected parts of the data, + // and might offer guidance on how users can address the issues. + // + // error_message is an optional field. An error_message with an empty value + // is equivalent of it not being set. + string error_message = 2; } diff --git a/opentelemetry/proto/collector/metrics/v1/metrics_service.proto b/opentelemetry/proto/collector/metrics/v1/metrics_service.proto index a013d2e77..3a9ef69c1 100644 --- a/opentelemetry/proto/collector/metrics/v1/metrics_service.proto +++ b/opentelemetry/proto/collector/metrics/v1/metrics_service.proto @@ -43,4 +43,35 @@ message ExportMetricsServiceRequest { } message ExportMetricsServiceResponse { + // The details of a partially successful export request. + // + // If the request is only partially accepted + // (i.e. when the server accepts only parts of the data and rejects the rest) + // the server MUST initialize the `partial_success` field and MUST + // set the `rejected_data_points` with the number of data points it rejected. + // + // The server MUST NOT set the `partial_success` field when the + // request was accepted in its entirety. + ExportMetricsPartialSuccess partial_success = 1; +} + +message ExportMetricsPartialSuccess { + // The number of rejected data points. + // + // If the request is only partially accepted, the server MUST set + // the `rejected_data_points` field. + // + // Given the server should only initialize the `partial_success` field + // when it rejects parts of the data, a `rejected_data_points` with + // value = '0' is considered an invalid result. Senders should + // interpret such responses from a server as if all data points were accepted. + int64 rejected_data_points = 1; + + // A developer-facing human-readable error message in English. It should + // explain why the server rejected parts of the data, + // and might offer guidance on how users can address the issues. + // + // error_message is an optional field. An error_message with an empty value + // is equivalent of it not being set. + string error_message = 2; } diff --git a/opentelemetry/proto/collector/trace/v1/trace_service.proto b/opentelemetry/proto/collector/trace/v1/trace_service.proto index 75d694294..3aeec8dc5 100644 --- a/opentelemetry/proto/collector/trace/v1/trace_service.proto +++ b/opentelemetry/proto/collector/trace/v1/trace_service.proto @@ -43,4 +43,35 @@ message ExportTraceServiceRequest { } message ExportTraceServiceResponse { + // The details of a partially successful export request. + // + // If the request is only partially accepted + // (i.e. when the server accepts only parts of the data and rejects the rest) + // the server MUST initialize the `partial_success` field and MUST + // set the `rejected_spans` with the number of spans it rejected. + // + // The server MUST NOT set the `partial_success` field when the + // request was accepted in its entirety. + ExportTracePartialSuccess partial_success = 1; +} + +message ExportTracePartialSuccess { + // The number of rejected spans. + // + // If the request is only partially accepted, the server MUST set + // the `rejected_spans` field. + // + // Given the server should only initialize the `partial_success` field + // when it rejects parts of the data, a `rejected_spans` with + // value = '0' is considered an invalid result. Senders should + // interpret such responses from a server as if all spans were accepted. + int64 rejected_spans = 1; + + // A developer-facing human-readable error message in English. It should + // explain why the server rejected parts of the data, + // and might offer guidance on how users can address the issues. + // + // error_message is an optional field. An error_message with an empty value + // is equivalent of it not being set. + string error_message = 2; } From ffb856d885431a60fa96992acb57aa36b8bb4079 Mon Sep 17 00:00:00 2001 From: Joao Grassi Date: Fri, 22 Jul 2022 12:21:57 +0200 Subject: [PATCH 2/6] Update changelog with PR number --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9419507c0..ab0e97ce6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,7 @@ Full list of differences found in [this compare](https://github.com/open-telemet * Introduce Scope Attributes. [#395](https://github.com/open-telemetry/opentelemetry-proto/pull/395) * Introduce partial success fields in `ExportServiceResponse`. - [#?](https://github.com/open-telemetry/opentelemetry-proto/pull/?) + [#414](https://github.com/open-telemetry/opentelemetry-proto/pull/414) ### Removed From 32ab10ec580906780e6cedb47f362c8d35be83f1 Mon Sep 17 00:00:00 2001 From: Joao Grassi Date: Tue, 26 Jul 2022 10:19:18 +0200 Subject: [PATCH 3/6] PR suggestions --- opentelemetry/proto/collector/logs/v1/logs_service.proto | 7 ++----- .../proto/collector/metrics/v1/metrics_service.proto | 7 ++----- opentelemetry/proto/collector/trace/v1/trace_service.proto | 7 ++----- 3 files changed, 6 insertions(+), 15 deletions(-) diff --git a/opentelemetry/proto/collector/logs/v1/logs_service.proto b/opentelemetry/proto/collector/logs/v1/logs_service.proto index b98613e1d..1e8b7f5b7 100644 --- a/opentelemetry/proto/collector/logs/v1/logs_service.proto +++ b/opentelemetry/proto/collector/logs/v1/logs_service.proto @@ -57,13 +57,10 @@ message ExportLogsServiceResponse { message ExportLogsPartialSuccess { // The number of rejected log records. - // - // If the request is only partially accepted, the server MUST set - // the `rejected_log_records` field. // - // Given the server should only initialize the `partial_success` field + // Given the server MUST only initialize the `partial_success` field // when it rejects parts of the data, a `rejected_log_records` with - // value = '0' is considered an invalid result. Senders should + // value = '0' is considered an invalid result. Senders MUST // interpret such responses from a server as if all log records were accepted. int64 rejected_log_records = 1; diff --git a/opentelemetry/proto/collector/metrics/v1/metrics_service.proto b/opentelemetry/proto/collector/metrics/v1/metrics_service.proto index 3a9ef69c1..1356f5f81 100644 --- a/opentelemetry/proto/collector/metrics/v1/metrics_service.proto +++ b/opentelemetry/proto/collector/metrics/v1/metrics_service.proto @@ -58,12 +58,9 @@ message ExportMetricsServiceResponse { message ExportMetricsPartialSuccess { // The number of rejected data points. // - // If the request is only partially accepted, the server MUST set - // the `rejected_data_points` field. - // - // Given the server should only initialize the `partial_success` field + // Given the server MUST only initialize the `partial_success` field // when it rejects parts of the data, a `rejected_data_points` with - // value = '0' is considered an invalid result. Senders should + // value = '0' is considered an invalid result. Senders MUST // interpret such responses from a server as if all data points were accepted. int64 rejected_data_points = 1; diff --git a/opentelemetry/proto/collector/trace/v1/trace_service.proto b/opentelemetry/proto/collector/trace/v1/trace_service.proto index 3aeec8dc5..6bb3683b3 100644 --- a/opentelemetry/proto/collector/trace/v1/trace_service.proto +++ b/opentelemetry/proto/collector/trace/v1/trace_service.proto @@ -58,12 +58,9 @@ message ExportTraceServiceResponse { message ExportTracePartialSuccess { // The number of rejected spans. // - // If the request is only partially accepted, the server MUST set - // the `rejected_spans` field. - // - // Given the server should only initialize the `partial_success` field + // Given the server MUST only initialize the `partial_success` field // when it rejects parts of the data, a `rejected_spans` with - // value = '0' is considered an invalid result. Senders should + // value = '0' is considered an invalid result. Senders MUST // interpret such responses from a server as if all spans were accepted. int64 rejected_spans = 1; From 3461d1f15a4a12b0bab5c567f22b0030b72e070e Mon Sep 17 00:00:00 2001 From: Joao Grassi Date: Mon, 1 Aug 2022 14:22:37 +0200 Subject: [PATCH 4/6] Improve docs around using rejected as 0 to convey warnings --- .../collector/logs/v1/logs_service.proto | 21 ++++++++++--------- .../metrics/v1/metrics_service.proto | 21 ++++++++++--------- .../collector/trace/v1/trace_service.proto | 21 ++++++++++--------- 3 files changed, 33 insertions(+), 30 deletions(-) diff --git a/opentelemetry/proto/collector/logs/v1/logs_service.proto b/opentelemetry/proto/collector/logs/v1/logs_service.proto index 1e8b7f5b7..7159987c1 100644 --- a/opentelemetry/proto/collector/logs/v1/logs_service.proto +++ b/opentelemetry/proto/collector/logs/v1/logs_service.proto @@ -48,25 +48,26 @@ message ExportLogsServiceResponse { // If the request is only partially accepted // (i.e. when the server accepts only parts of the data and rejects the rest) // the server MUST initialize the `partial_success` field and MUST - // set the `rejected_log_records` with the number of log records it rejected. + // set the `rejected_` with the number of items it rejected. // - // The server MUST NOT set the `partial_success` field when the - // request was accepted in its entirety. + // Servers MAY also make use of the `partial_success` field to convey + // warnings/suggestions to senders when the request was fully accepted. + // In such cases, the `rejected_` MUST have a value of `0` and + // the `error_message` MUST be non-empty. ExportLogsPartialSuccess partial_success = 1; } message ExportLogsPartialSuccess { // The number of rejected log records. // - // Given the server MUST only initialize the `partial_success` field - // when it rejects parts of the data, a `rejected_log_records` with - // value = '0' is considered an invalid result. Senders MUST - // interpret such responses from a server as if all log records were accepted. + // A `rejected_` field holding a `0` value indicates that the + // request was fully accepted. int64 rejected_log_records = 1; - // A developer-facing human-readable error message in English. It should - // explain why the server rejected parts of the data, - // and might offer guidance on how users can address the issues. + // A developer-facing human-readable message in English. It should be used + // either to explain why the server rejected parts of the data during a partial + // success or to convey warnings/suggestions during a full success. The message + // should offer guidance on how users can address such issues. // // error_message is an optional field. An error_message with an empty value // is equivalent of it not being set. diff --git a/opentelemetry/proto/collector/metrics/v1/metrics_service.proto b/opentelemetry/proto/collector/metrics/v1/metrics_service.proto index 1356f5f81..e729f77a9 100644 --- a/opentelemetry/proto/collector/metrics/v1/metrics_service.proto +++ b/opentelemetry/proto/collector/metrics/v1/metrics_service.proto @@ -48,25 +48,26 @@ message ExportMetricsServiceResponse { // If the request is only partially accepted // (i.e. when the server accepts only parts of the data and rejects the rest) // the server MUST initialize the `partial_success` field and MUST - // set the `rejected_data_points` with the number of data points it rejected. + // set the `rejected_` with the number of items it rejected. // - // The server MUST NOT set the `partial_success` field when the - // request was accepted in its entirety. + // Servers MAY also make use of the `partial_success` field to convey + // warnings/suggestions to senders when the request was fully accepted. + // In such cases, the `rejected_` MUST have a value of `0` and + // the `error_message` MUST be non-empty. ExportMetricsPartialSuccess partial_success = 1; } message ExportMetricsPartialSuccess { // The number of rejected data points. // - // Given the server MUST only initialize the `partial_success` field - // when it rejects parts of the data, a `rejected_data_points` with - // value = '0' is considered an invalid result. Senders MUST - // interpret such responses from a server as if all data points were accepted. + // A `rejected_` field holding a `0` value indicates that the + // request was fully accepted. int64 rejected_data_points = 1; - // A developer-facing human-readable error message in English. It should - // explain why the server rejected parts of the data, - // and might offer guidance on how users can address the issues. + // A developer-facing human-readable message in English. It should be used + // either to explain why the server rejected parts of the data during a partial + // success or to convey warnings/suggestions during a full success. The message + // should offer guidance on how users can address such issues. // // error_message is an optional field. An error_message with an empty value // is equivalent of it not being set. diff --git a/opentelemetry/proto/collector/trace/v1/trace_service.proto b/opentelemetry/proto/collector/trace/v1/trace_service.proto index 6bb3683b3..ccaac00b3 100644 --- a/opentelemetry/proto/collector/trace/v1/trace_service.proto +++ b/opentelemetry/proto/collector/trace/v1/trace_service.proto @@ -48,25 +48,26 @@ message ExportTraceServiceResponse { // If the request is only partially accepted // (i.e. when the server accepts only parts of the data and rejects the rest) // the server MUST initialize the `partial_success` field and MUST - // set the `rejected_spans` with the number of spans it rejected. + // set the `rejected_` with the number of items it rejected. // - // The server MUST NOT set the `partial_success` field when the - // request was accepted in its entirety. + // Servers MAY also make use of the `partial_success` field to convey + // warnings/suggestions to senders when the request was fully accepted. + // In such cases, the `rejected_` MUST have a value of `0` and + // the `error_message` MUST be non-empty. ExportTracePartialSuccess partial_success = 1; } message ExportTracePartialSuccess { // The number of rejected spans. // - // Given the server MUST only initialize the `partial_success` field - // when it rejects parts of the data, a `rejected_spans` with - // value = '0' is considered an invalid result. Senders MUST - // interpret such responses from a server as if all spans were accepted. + // A `rejected_` field holding a `0` value indicates that the + // request was fully accepted. int64 rejected_spans = 1; - // A developer-facing human-readable error message in English. It should - // explain why the server rejected parts of the data, - // and might offer guidance on how users can address the issues. + // A developer-facing human-readable message in English. It should be used + // either to explain why the server rejected parts of the data during a partial + // success or to convey warnings/suggestions during a full success. The message + // should offer guidance on how users can address such issues. // // error_message is an optional field. An error_message with an empty value // is equivalent of it not being set. From b63cda7f68631d93af832620ee4e9c52f4a36775 Mon Sep 17 00:00:00 2001 From: Joao Grassi Date: Tue, 2 Aug 2022 17:39:31 +0200 Subject: [PATCH 5/6] Adapt comments to match recent spec wording changes --- opentelemetry/proto/collector/logs/v1/logs_service.proto | 2 +- opentelemetry/proto/collector/metrics/v1/metrics_service.proto | 2 +- opentelemetry/proto/collector/trace/v1/trace_service.proto | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/opentelemetry/proto/collector/logs/v1/logs_service.proto b/opentelemetry/proto/collector/logs/v1/logs_service.proto index 7159987c1..ec774430d 100644 --- a/opentelemetry/proto/collector/logs/v1/logs_service.proto +++ b/opentelemetry/proto/collector/logs/v1/logs_service.proto @@ -51,7 +51,7 @@ message ExportLogsServiceResponse { // set the `rejected_` with the number of items it rejected. // // Servers MAY also make use of the `partial_success` field to convey - // warnings/suggestions to senders when the request was fully accepted. + // warnings/suggestions to senders even when the request was fully accepted. // In such cases, the `rejected_` MUST have a value of `0` and // the `error_message` MUST be non-empty. ExportLogsPartialSuccess partial_success = 1; diff --git a/opentelemetry/proto/collector/metrics/v1/metrics_service.proto b/opentelemetry/proto/collector/metrics/v1/metrics_service.proto index e729f77a9..99d3cb6a6 100644 --- a/opentelemetry/proto/collector/metrics/v1/metrics_service.proto +++ b/opentelemetry/proto/collector/metrics/v1/metrics_service.proto @@ -51,7 +51,7 @@ message ExportMetricsServiceResponse { // set the `rejected_` with the number of items it rejected. // // Servers MAY also make use of the `partial_success` field to convey - // warnings/suggestions to senders when the request was fully accepted. + // warnings/suggestions to senders even when the request was fully accepted. // In such cases, the `rejected_` MUST have a value of `0` and // the `error_message` MUST be non-empty. ExportMetricsPartialSuccess partial_success = 1; diff --git a/opentelemetry/proto/collector/trace/v1/trace_service.proto b/opentelemetry/proto/collector/trace/v1/trace_service.proto index ccaac00b3..64dc613ea 100644 --- a/opentelemetry/proto/collector/trace/v1/trace_service.proto +++ b/opentelemetry/proto/collector/trace/v1/trace_service.proto @@ -51,7 +51,7 @@ message ExportTraceServiceResponse { // set the `rejected_` with the number of items it rejected. // // Servers MAY also make use of the `partial_success` field to convey - // warnings/suggestions to senders when the request was fully accepted. + // warnings/suggestions to senders even when the request was fully accepted. // In such cases, the `rejected_` MUST have a value of `0` and // the `error_message` MUST be non-empty. ExportTracePartialSuccess partial_success = 1; From 49bcec06f20a67f28b77894ebd23591b2432360f Mon Sep 17 00:00:00 2001 From: Joao Grassi Date: Tue, 2 Aug 2022 18:37:28 +0200 Subject: [PATCH 6/6] Clarify behavior of partial success not set/empty --- opentelemetry/proto/collector/logs/v1/logs_service.proto | 8 ++++++-- .../proto/collector/metrics/v1/metrics_service.proto | 6 +++++- .../proto/collector/trace/v1/trace_service.proto | 6 +++++- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/opentelemetry/proto/collector/logs/v1/logs_service.proto b/opentelemetry/proto/collector/logs/v1/logs_service.proto index ec774430d..8260d8aae 100644 --- a/opentelemetry/proto/collector/logs/v1/logs_service.proto +++ b/opentelemetry/proto/collector/logs/v1/logs_service.proto @@ -54,12 +54,16 @@ message ExportLogsServiceResponse { // warnings/suggestions to senders even when the request was fully accepted. // In such cases, the `rejected_` MUST have a value of `0` and // the `error_message` MUST be non-empty. + // + // A `partial_success` message with an empty value (rejected_ = 0 and + // `error_message` = "") is equivalent to it not being set/present. Senders + // SHOULD interpret it the same way as in the full success case. ExportLogsPartialSuccess partial_success = 1; } message ExportLogsPartialSuccess { // The number of rejected log records. - // + // // A `rejected_` field holding a `0` value indicates that the // request was fully accepted. int64 rejected_log_records = 1; @@ -70,6 +74,6 @@ message ExportLogsPartialSuccess { // should offer guidance on how users can address such issues. // // error_message is an optional field. An error_message with an empty value - // is equivalent of it not being set. + // is equivalent to it not being set. string error_message = 2; } diff --git a/opentelemetry/proto/collector/metrics/v1/metrics_service.proto b/opentelemetry/proto/collector/metrics/v1/metrics_service.proto index 99d3cb6a6..dd48f1ad3 100644 --- a/opentelemetry/proto/collector/metrics/v1/metrics_service.proto +++ b/opentelemetry/proto/collector/metrics/v1/metrics_service.proto @@ -54,6 +54,10 @@ message ExportMetricsServiceResponse { // warnings/suggestions to senders even when the request was fully accepted. // In such cases, the `rejected_` MUST have a value of `0` and // the `error_message` MUST be non-empty. + // + // A `partial_success` message with an empty value (rejected_ = 0 and + // `error_message` = "") is equivalent to it not being set/present. Senders + // SHOULD interpret it the same way as in the full success case. ExportMetricsPartialSuccess partial_success = 1; } @@ -70,6 +74,6 @@ message ExportMetricsPartialSuccess { // should offer guidance on how users can address such issues. // // error_message is an optional field. An error_message with an empty value - // is equivalent of it not being set. + // is equivalent to it not being set. string error_message = 2; } diff --git a/opentelemetry/proto/collector/trace/v1/trace_service.proto b/opentelemetry/proto/collector/trace/v1/trace_service.proto index 64dc613ea..d6fe67f9e 100644 --- a/opentelemetry/proto/collector/trace/v1/trace_service.proto +++ b/opentelemetry/proto/collector/trace/v1/trace_service.proto @@ -54,6 +54,10 @@ message ExportTraceServiceResponse { // warnings/suggestions to senders even when the request was fully accepted. // In such cases, the `rejected_` MUST have a value of `0` and // the `error_message` MUST be non-empty. + // + // A `partial_success` message with an empty value (rejected_ = 0 and + // `error_message` = "") is equivalent to it not being set/present. Senders + // SHOULD interpret it the same way as in the full success case. ExportTracePartialSuccess partial_success = 1; } @@ -70,6 +74,6 @@ message ExportTracePartialSuccess { // should offer guidance on how users can address such issues. // // error_message is an optional field. An error_message with an empty value - // is equivalent of it not being set. + // is equivalent to it not being set. string error_message = 2; }