From ccf861f8c4813d9a8b93d7a5eeadd7cc9daa8b3f Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Mon, 21 Oct 2024 13:42:58 -0500 Subject: [PATCH 1/5] Clarify null vs empty --- specification/configuration/sdk.md | 33 ++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/specification/configuration/sdk.md b/specification/configuration/sdk.md index 7baa0e18367..69fbe826b5b 100644 --- a/specification/configuration/sdk.md +++ b/specification/configuration/sdk.md @@ -179,7 +179,23 @@ Parse and validate a [configuration file](./data-model.md#file-based-configurati Parse MUST perform [environment variable substitution](./data-model.md#environment-variable-substitution). -Parse MUST interpret null as equivalent to unset. +Parse MUST differentiate between properties that are unset and present but null. +For example, consider the following snippet: + +```yaml +meter_provider: + views: + - selector: + name: some.metric.name + stream: + aggregation: + drop: +``` + +As a result, the view stream should be configured with the `drop` aggregation. +Note that some aggregations have additional arguments, but `drop` does not. The +user MUST not be required to specify an empty object (i.e. `drop: {}`) in these +cases. When encountering a reference to a [SDK extension component](#sdk-extension-components) which is not built in to @@ -212,14 +228,15 @@ Interpret configuration model and return SDK components. The multiple responses MAY be returned using a tuple, or some other data structure encapsulating the components. -If a field is null or unset and a default value is defined, Create MUST ensure -the SDK component is configured with the default value. If a field is null or -unset and no default value is defined, Create SHOULD return an error. For -example, if configuring +If a field has a default value defined (i.e. is _not_ required) and is present +but null or unset, Create MUST ensure the SDK component is configured with the +default value. If a field is required and is present but null or unset, Create +SHOULD return an error. For example, if configuring the [span batching processor](../trace/sdk.md#batching-processor) and -the `scheduleDelayMillis` field is null or unset, the component is configured -with the default value of `5000`. However, if the `exporter` field is null or -unset, Create fails fast since there is no default value for `exporter`. +the `scheduleDelayMillis` field is present but null or unset, the component is +configured with the default value of `5000`. However, if the `exporter` field is +present but null or unset, Create fails fast since there is no default value +for `exporter`. When encountering a reference to a [SDK extension component](#sdk-extension-components) which is not built in to From fb0df9b0ff83a61cef098184f2c0d8ac123d3b9a Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Tue, 22 Oct 2024 09:21:07 -0500 Subject: [PATCH 2/5] Add changelog entry --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba4d9e6a111..4c854ed3264 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,9 @@ release. ### SDK Configuration +- Clarify declarative configuration parse requirements for null vs empty. + ([#4269](https://github.com/open-telemetry/opentelemetry-specification/pull/4269)) + ### Common ### Supplementary Guidelines From caa6e6089de9dd8ab5e1411849a36c0df6c865f0 Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Thu, 24 Oct 2024 09:34:51 -0500 Subject: [PATCH 3/5] Rename field -> property --- specification/configuration/data-model.md | 2 +- specification/configuration/sdk.md | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/specification/configuration/data-model.md b/specification/configuration/data-model.md index 6b7abcac269..4b3f9bfc52a 100644 --- a/specification/configuration/data-model.md +++ b/specification/configuration/data-model.md @@ -86,7 +86,7 @@ results are allowed) and an error describing the parse failure to the user. Node types MUST be interpreted after environment variable substitution takes place. This ensures the environment string representation of boolean, integer, -or floating point fields can be properly converted to expected types. +or floating point properties can be properly converted to expected types. It MUST NOT be possible to inject YAML structures by environment variables. For example, see references to `INVALID_MAP_VALUE` environment variable below. diff --git a/specification/configuration/sdk.md b/specification/configuration/sdk.md index 69fbe826b5b..626488276c6 100644 --- a/specification/configuration/sdk.md +++ b/specification/configuration/sdk.md @@ -228,15 +228,15 @@ Interpret configuration model and return SDK components. The multiple responses MAY be returned using a tuple, or some other data structure encapsulating the components. -If a field has a default value defined (i.e. is _not_ required) and is present -but null or unset, Create MUST ensure the SDK component is configured with the -default value. If a field is required and is present but null or unset, Create -SHOULD return an error. For example, if configuring +If a property has a default value defined (i.e. is _not_ required) and is +present but null or unset, Create MUST ensure the SDK component is configured +with the default value. If a property is required and is present but null or +unset, Create SHOULD return an error. For example, if configuring the [span batching processor](../trace/sdk.md#batching-processor) and -the `scheduleDelayMillis` field is present but null or unset, the component is -configured with the default value of `5000`. However, if the `exporter` field is -present but null or unset, Create fails fast since there is no default value -for `exporter`. +the `scheduleDelayMillis` property is present but null or unset, the component +is configured with the default value of `5000`. However, if the `exporter` +property is present but null or unset, Create fails fast since there is no +default value for `exporter`. When encountering a reference to a [SDK extension component](#sdk-extension-components) which is not built in to From 1d7cbc339bf784dbb4de0cc3180e1bc0d0e57035 Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Thu, 24 Oct 2024 09:39:41 -0500 Subject: [PATCH 4/5] Clarify example --- specification/configuration/sdk.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/specification/configuration/sdk.md b/specification/configuration/sdk.md index 626488276c6..0e2bd5ed608 100644 --- a/specification/configuration/sdk.md +++ b/specification/configuration/sdk.md @@ -180,7 +180,9 @@ Parse and validate a [configuration file](./data-model.md#file-based-configurati Parse MUST perform [environment variable substitution](./data-model.md#environment-variable-substitution). Parse MUST differentiate between properties that are unset and present but null. -For example, consider the following snippet: +For example, consider the following snippet, +noting `.meter_provider.views[0].stream.drop` is set to null rather than an +empty object: ```yaml meter_provider: From bd2467632a394b9e13eb58e63c23b6090389ba16 Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Fri, 25 Oct 2024 16:44:04 -0500 Subject: [PATCH 5/5] Rephrase missing vs present but null --- specification/configuration/sdk.md | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/specification/configuration/sdk.md b/specification/configuration/sdk.md index 0e2bd5ed608..25024edecea 100644 --- a/specification/configuration/sdk.md +++ b/specification/configuration/sdk.md @@ -179,10 +179,9 @@ Parse and validate a [configuration file](./data-model.md#file-based-configurati Parse MUST perform [environment variable substitution](./data-model.md#environment-variable-substitution). -Parse MUST differentiate between properties that are unset and present but null. -For example, consider the following snippet, -noting `.meter_provider.views[0].stream.drop` is set to null rather than an -empty object: +Parse MUST differentiate between properties that are missing and properties that +are present but null. For example, consider the following snippet, +noting `.meter_provider.views[0].stream.drop` is present but null: ```yaml meter_provider: @@ -231,13 +230,13 @@ The multiple responses MAY be returned using a tuple, or some other data structure encapsulating the components. If a property has a default value defined (i.e. is _not_ required) and is -present but null or unset, Create MUST ensure the SDK component is configured -with the default value. If a property is required and is present but null or -unset, Create SHOULD return an error. For example, if configuring +missing or present but null, Create MUST ensure the SDK component is configured +with the default value. If a property is required and is missing or present but +null, Create SHOULD return an error. For example, if configuring the [span batching processor](../trace/sdk.md#batching-processor) and -the `scheduleDelayMillis` property is present but null or unset, the component +the `scheduleDelayMillis` property is missing or present but null, the component is configured with the default value of `5000`. However, if the `exporter` -property is present but null or unset, Create fails fast since there is no +property is missing or present but null, Create fails fast since there is no default value for `exporter`. When encountering a reference to