Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move the evaluated value from the event body to attributes #1990

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions .chloggen/ff-value-body-attribute.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Use this changelog template to create an entry for release notes.
#
# If your change doesn't affect end users you should instead start
# your pull request title with [chore] or use the "Skip Changelog" label.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: breaking

# The name of the area of concern in the attributes-registry, (e.g. http, cloud, db)
component: feature_flag

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Move the evaluated value from the event body to attributes

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
# The values here must be integers.
issues: [1990]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:
13 changes: 13 additions & 0 deletions docs/attributes-registry/feature-flag.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ This document defines attributes for Feature Flags.
| <a id="feature-flag-evaluation-reason" href="#feature-flag-evaluation-reason">`feature_flag.evaluation.reason`</a> | string | The reason code which shows how a feature flag value was determined. | `static`; `targeting_match`; `error`; `default` | ![Development](https://img.shields.io/badge/-development-blue) |
| <a id="feature-flag-key" href="#feature-flag-key">`feature_flag.key`</a> | string | The lookup key of the feature flag. | `logo-color` | ![Development](https://img.shields.io/badge/-development-blue) |
| <a id="feature-flag-provider-name" href="#feature-flag-provider-name">`feature_flag.provider_name`</a> | string | Identifies the feature flag provider. | `Flag Manager` | ![Development](https://img.shields.io/badge/-development-blue) |
| <a id="feature-flag-result-type" href="#feature-flag-result-type">`feature_flag.result.type`</a> | string | The JSON type of the `feature_flag.result.value`. | `string`; `boolean`; `number` | ![Development](https://img.shields.io/badge/-development-blue) |
| <a id="feature-flag-result-value" href="#feature-flag-result-value">`feature_flag.result.value`</a> | string | The evaluated value of the feature flag represented as a JSON string. | `#ff0000`; `1`; `true` | ![Development](https://img.shields.io/badge/-development-blue) |
| <a id="feature-flag-set-id" href="#feature-flag-set-id">`feature_flag.set.id`</a> | string | The identifier of the [flag set](https://openfeature.dev/specification/glossary/#flag-set) to which the feature flag belongs. | `proj-1`; `ab98sgs`; `service1/dev` | ![Development](https://img.shields.io/badge/-development-blue) |
| <a id="feature-flag-variant" href="#feature-flag-variant">`feature_flag.variant`</a> | string | A semantic identifier for an evaluated flag value. [1] | `red`; `true`; `on` | ![Development](https://img.shields.io/badge/-development-blue) |
| <a id="feature-flag-version" href="#feature-flag-version">`feature_flag.version`</a> | string | The version of the ruleset used during the evaluation. This may be any stable value which uniquely identifies the ruleset. | `1`; `01ABCDEF` | ![Development](https://img.shields.io/badge/-development-blue) |
Expand All @@ -38,3 +40,14 @@ For example, the variant `red` maybe be used for the value `#c05543`.
| `static` | The resolved value is static (no dynamic evaluation). | ![Development](https://img.shields.io/badge/-development-blue) |
| `targeting_match` | The resolved value was the result of a dynamic evaluation, such as a rule or specific user-targeting. | ![Development](https://img.shields.io/badge/-development-blue) |
| `unknown` | The reason for the resolved value could not be determined. | ![Development](https://img.shields.io/badge/-development-blue) |

---

`feature_flag.result.type` has the following list of well-known values. If one of them applies, then the respective value MUST be used; otherwise, a custom value MAY be used.

| Value | Description | Stability |
|---|---|---|
| `boolean` | The `feature_flag.result.value` is a stringified boolean. | ![Development](https://img.shields.io/badge/-development-blue) |
| `number` | The `feature_flag.result.value` is a stringified number. | ![Development](https://img.shields.io/badge/-development-blue) |
| `object` | The `feature_flag.result.value` is a stringified object. | ![Development](https://img.shields.io/badge/-development-blue) |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An object may be massive. Should we be worried about size or leave that up to the implementor?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you mention this in the notes for this attribute? we discussed large attributes in the log/event SIG just yesterday as this is also an issue coming up in GenAI events. we currently want these to (generally) go into event attributes, and are exploring ways to indicate these (potentially) large attributes to pipelines/backends so they can choose alternative storage for them if they want

there's also a case to be made for this to remain in the body if it's going to be large and if it's the central thing reported by the event. e.g. we are still debating whether mobile crash dumps should be reported as an event attribute or event body

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can add a note. I think it's not expected that it is often large, but it can be. In that case is attribute still appropriate?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it's going to be large

I think it's only sometimes large

and if it's the central thing reported by the event

The evaluation result is obviously important, but we expect variant (a short name that identifies a result) to be used more often. Some systems don't have a concept of variant though

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

| `string` | The `feature_flag.result.value` is a string. | ![Development](https://img.shields.io/badge/-development-blue) |
29 changes: 19 additions & 10 deletions docs/feature-flags/feature-flags-logs.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,11 @@ A `feature_flag.evaluation` event SHOULD be emitted whenever a feature flag valu
|---|---|---|---|---|---|
| [`feature_flag.key`](/docs/attributes-registry/feature-flag.md) | string | The lookup key of the feature flag. | `logo-color` | `Required` | ![Development](https://img.shields.io/badge/-development-blue) |
| [`error.type`](/docs/attributes-registry/error.md) | string | Describes a class of error the operation ended with. [1] | `provider_not_ready`; `targeting_key_missing`; `provider_fatal`; `general` | `Conditionally Required` [2] | ![Stable](https://img.shields.io/badge/-stable-lightgreen) |
| [`feature_flag.variant`](/docs/attributes-registry/feature-flag.md) | string | A semantic identifier for an evaluated flag value. [3] | `red`; `true`; `on` | `Conditionally Required` [4] | ![Development](https://img.shields.io/badge/-development-blue) |
| [`feature_flag.result.type`](/docs/attributes-registry/feature-flag.md) | string | The JSON type of the `feature_flag.result.value`. | `string`; `boolean`; `number` | `Conditionally Required` [3] | ![Development](https://img.shields.io/badge/-development-blue) |
| [`feature_flag.result.value`](/docs/attributes-registry/feature-flag.md) | string | The evaluated value of the feature flag represented as a JSON string. | `#ff0000`; `1`; `true` | `Conditionally Required` [4] | ![Development](https://img.shields.io/badge/-development-blue) |
| [`feature_flag.variant`](/docs/attributes-registry/feature-flag.md) | string | A semantic identifier for an evaluated flag value. [5] | `red`; `true`; `on` | `Conditionally Required` [6] | ![Development](https://img.shields.io/badge/-development-blue) |
| [`feature_flag.context.id`](/docs/attributes-registry/feature-flag.md) | string | The unique identifier for the flag evaluation context. For example, the targeting key. | `5157782b-2203-4c80-a857-dbbd5e7761db` | `Recommended` | ![Development](https://img.shields.io/badge/-development-blue) |
| [`feature_flag.evaluation.error.message`](/docs/attributes-registry/feature-flag.md) | string | A message explaining the nature of an error occurring during flag evaluation. | `Flag `header-color` expected type `string` but found type `number`` | `Recommended` [5] | ![Development](https://img.shields.io/badge/-development-blue) |
| [`feature_flag.evaluation.error.message`](/docs/attributes-registry/feature-flag.md) | string | A message explaining the nature of an error occurring during flag evaluation. | `Flag `header-color` expected type `string` but found type `number`` | `Recommended` [7] | ![Development](https://img.shields.io/badge/-development-blue) |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the be evaluation or result? It seems odd to me to use result for success and evaluation for failure.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be (generic) error.message, which we don't have, but I would support.

It's essentially what we put in span status description on spans, but don't have a place today to put on events.

@lmolkova wdyt?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left it as evaluation because in a failure scenario you might not actually have a result. its the evaluation that fails

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created a PR for discussion about generic error.message #1992

| [`feature_flag.evaluation.reason`](/docs/attributes-registry/feature-flag.md) | string | The reason code which shows how a feature flag value was determined. | `static`; `targeting_match`; `error`; `default` | `Recommended` | ![Development](https://img.shields.io/badge/-development-blue) |
| [`feature_flag.provider_name`](/docs/attributes-registry/feature-flag.md) | string | Identifies the feature flag provider. | `Flag Manager` | `Recommended` | ![Development](https://img.shields.io/badge/-development-blue) |
| [`feature_flag.set.id`](/docs/attributes-registry/feature-flag.md) | string | The identifier of the [flag set](https://openfeature.dev/specification/glossary/#flag-set) to which the feature flag belongs. | `proj-1`; `ab98sgs`; `service1/dev` | `Recommended` | ![Development](https://img.shields.io/badge/-development-blue) |
Expand All @@ -83,14 +85,18 @@ A `feature_flag.evaluation` event SHOULD be emitted whenever a feature flag valu

**[2] `error.type`:** If and only if an error occurred during flag evaluation.

**[3] `feature_flag.variant`:** A semantic identifier, commonly referred to as a variant, provides a means
**[3] `feature_flag.result.type`:** If and only if `feature_flag.result.value` is set.

**[4] `feature_flag.result.value`:** If and only if feature flag provider does not supply variant or equivalent concept. Otherwise, `feature_flag.result.value` should be treated as opt-in.

**[5] `feature_flag.variant`:** A semantic identifier, commonly referred to as a variant, provides a means
for referring to a value without including the value itself. This can
provide additional context for understanding the meaning behind a value.
For example, the variant `red` maybe be used for the value `#c05543`.

**[4] `feature_flag.variant`:** If feature flag provider supplies a variant or equivalent concept.
**[6] `feature_flag.variant`:** If feature flag provider supplies a variant or equivalent concept.

**[5] `feature_flag.evaluation.error.message`:** If and only if an error occurred. It's NOT RECOMMENDED to duplicate the value of `error.type` in `feature_flag.evaluation.error.message`.
**[7] `feature_flag.evaluation.error.message`:** If and only if an error occurred. It's NOT RECOMMENDED to duplicate the value of `error.type` in `feature_flag.evaluation.error.message`.

---

Expand All @@ -116,13 +122,16 @@ For example, the variant `red` maybe be used for the value `#c05543`.
| `targeting_match` | The resolved value was the result of a dynamic evaluation, such as a rule or specific user-targeting. | ![Development](https://img.shields.io/badge/-development-blue) |
| `unknown` | The reason for the resolved value could not be determined. | ![Development](https://img.shields.io/badge/-development-blue) |

**Body fields:**
---

| Body Field | Type | Description | Examples | [Requirement Level](https://opentelemetry.io/docs/specs/semconv/general/attribute-requirement-level/) | Stability |
|---|---|---|---|---|---|
| `value` | undefined | The evaluated value of the feature flag. | `#ff0000`; `1`; `true` | `Conditionally Required` [1] | ![Development](https://img.shields.io/badge/-development-blue) |
`feature_flag.result.type` has the following list of well-known values. If one of them applies, then the respective value MUST be used; otherwise, a custom value MAY be used.

**[1] `value`:** If and only if feature flag provider does not supply variant or equivalent concept. Otherwise, `value` should be treated as opt-in.
| Value | Description | Stability |
|---|---|---|
| `boolean` | The `feature_flag.result.value` is a stringified boolean. | ![Development](https://img.shields.io/badge/-development-blue) |
| `number` | The `feature_flag.result.value` is a stringified number. | ![Development](https://img.shields.io/badge/-development-blue) |
| `object` | The `feature_flag.result.value` is a stringified object. | ![Development](https://img.shields.io/badge/-development-blue) |
| `string` | The `feature_flag.result.value` is a string. | ![Development](https://img.shields.io/badge/-development-blue) |

<!-- markdownlint-restore -->
<!-- prettier-ignore-end -->
Expand Down
24 changes: 9 additions & 15 deletions model/feature-flags/logs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,15 @@ groups:
- ref: feature_flag.variant
requirement_level:
conditionally_required: If feature flag provider supplies a variant or equivalent concept.
- ref: feature_flag.result.value
requirement_level:
conditionally_required: >
If and only if feature flag provider does not supply variant or equivalent concept.
Otherwise, `feature_flag.result.value` should be treated as opt-in.
- ref: feature_flag.result.type
requirement_level:
conditionally_required: >
If and only if `feature_flag.result.value` is set.
- ref: feature_flag.provider_name
requirement_level: recommended
- ref: feature_flag.context.id
Expand Down Expand Up @@ -49,18 +58,3 @@ groups:
- ref: feature_flag.evaluation.error.message
requirement_level:
recommended: If and only if an error occurred. It's NOT RECOMMENDED to duplicate the value of `error.type` in `feature_flag.evaluation.error.message`.
body:
id: feature_flag.evaluation
type: map
requirement_level: recommended
stability: development
fields:
- id: value
type: undefined
stability: development
brief: The evaluated value of the feature flag.
requirement_level:
conditionally_required: >
If and only if feature flag provider does not supply variant or equivalent concept.
Otherwise, `value` should be treated as opt-in.
examples: ["#ff0000", "1", "true"]
27 changes: 27 additions & 0 deletions model/feature-flags/registry.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,30 @@
stability: development
examples: ["Flag `header-color` expected type `string` but found type `number`"]
brief: A message explaining the nature of an error occurring during flag evaluation.
- id: feature_flag.result.value
type: string
stability: development
examples: ["#ff0000", "1", "true"]
brief: The evaluated value of the feature flag represented as a JSON string.
- id: feature_flag.result.type
type:
members:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm currently missing null which is a valid JSON type but is not a valid attribute. @trask what is generally the protocol here? Should I put anything technically representable? Or should I limit to what is allowed in attribute values?

- id: string
value: "string"
brief: The `feature_flag.result.value` is a string.
stability: development
- id: number
value: "number"
brief: The `feature_flag.result.value` is a stringified number.
stability: development
- id: boolean
value: "boolean"
brief: The `feature_flag.result.value` is a stringified boolean.
stability: development
- id: object
value: "object"
brief: The `feature_flag.result.value` is a stringified object.
stability: development
stability: development
examples: ["string", "boolean", "number"]
brief: The JSON type of the `feature_flag.result.value`.

Check failure on line 122 in model/feature-flags/registry.yaml

View workflow job for this annotation

GitHub Actions / yamllint

[new-line-at-end-of-file] no new line character at the end of file
Loading