open-feature / spec

OpenFeature specification
https://openfeature.dev
Apache License 2.0
639 stars 34 forks source link

Define OTel Semantic Convention for Feature Flagging #30

Closed beeme1mr closed 1 year ago

beeme1mr commented 2 years ago

OpenTelemetry has a concept of semantic conventions. This is a form of metadata that can be applied to a span. The attribute names should comply with their spec.

Here's an initial draft of properties that may be valuable.

Attribute Type Description Examples Required
feature_flag.flag_key string The unique identifier of the feature flag. show-new-logo Yes
feature_flag.provider.name string The name of the provider performing the flag evaluation. Flag Manager No
feature_flag.provider.management_url string The URL used to manage the feature flag in the provider. http://localhost:8080/flags/ No
feature_flag.evaluated.variant string The name associated with the evaluated value. [1] reverse See below
feature_flag.evaluated.value string A string representation of the evaluated value. [2] true See below

[1]: A variant should be used if it is available. If the variant is present, feature_flag.evaluated.value should be omitted.

[2]: The value should only be used if the variant is not available. How the value is represented as a string should be determined by the implementer.

Additional span attributes that already exist that may be useful in OpenFeature.

beeme1mr commented 2 years ago

It would be useful to also know the expected return type and the value. This becomes a bit tricky when dealing with JSON. In that case, we could either omit the property or create a hash.

moredip commented 2 years ago

feature_flag.flag_id or feature_flag.flag_name would be less ambiguous, IMHO. (rather than feature_flag.id)

moredip commented 2 years ago

Wouldn't we want to standardize an attribute that indicates the evaluated value of the flag?

feature_flag.evaluated_value, or just feature_flag.flag_value?

justinabrahms commented 2 years ago

The thing I'm missing here is "variant name".

In my world, we have a flag. Flags have rules. Rules will help you slot a user into a particular variant.

e.g. "redesign_enabled" flag might have variants "yes" and "no". I need that name in the OT output, as the value may change over time (and/or be big) but the names should be more stable.

moredip commented 2 years ago

The thing I'm missing here is "variant name".

@justinabrahms I think what I'm calling "evaluated value" you're calling "variant name", right? Not advocating for one over the other, just making sure I'm understanding your meaning

justinabrahms commented 2 years ago

@moredip "evaluated value" sounds like the resulting data type, I think. I want a name for that value type. So the value for "yes" would be true.

Another example:

feature name: header_order
rule: if they're in germany, give "wonky" order.
rule: if they're in england, give "reverse" order

variants:
wonky: 1,4,3,2
reverse: 4,3,2,1
default: 1,2,3,4

In this example, I want "reverse" to be sent to OTel, not "4,3,2,1". Does that help?

moredip commented 2 years ago

Oh thanks, the example helps! So "evaluated value" would be the actual value that the Application Author is getting back from the flag decision call, and "variant name" is the semantic name for that value?

justinabrahms commented 2 years ago

@moredip You've got it! I've attempted to document what I've said here: https://github.com/open-feature/spec/pull/31

toddbaert commented 2 years ago

@justinabrahms I made a similar comment in your recent PR: https://github.com/open-feature/spec/pull/31/files . I'll repeat it here:

While I see the distinction you're making between the flag value and the semantic name for that value, I'm not sure it can be meaningfully mapped to all (or even most) vendors.

How do we abstract this in vendors that don't really carry the "variant" concept?

justinabrahms commented 2 years ago

I'll keep the convo on that thread, but interested in solving that. :)

beeme1mr commented 2 years ago

feature_flag.flag_id or feature_flag.flag_name would be less ambiguous, IMHO. (rather than feature_flag.id)

@moredip, that makes sense. Unfortunately, there doesn't seem to be a clear term used by the industry. A quick look shows that name, key, and identifier is commonly used. I would prefer feature_flag.flag_id but feature_flag.flag_key would also make sense to me.

beeme1mr commented 2 years ago

Thanks @justinabrahms, I've updated the proposal based on your suggestions. I'm wondering if having the variant name would reduce or eliminate the need for the evaluation value itself. For now, I've left it in but perhaps it should be removed.

justinabrahms commented 2 years ago

I'm wondering if having the variant name would reduce or eliminate the need for the evaluation value itself.

I'm concerned about data size, but maybe we also need to know what was actually in there? Users may be able to update variant plums from value "yummy" to "Turns out, I don't like them" and the telemetry wouldn't know the difference otherwise. Is that important? I don't think it is important to my use-case, but may be to someone's.

beeme1mr commented 2 years ago

@justinabrahms, I agree that the variant name would be best in situations where the result is not a boolean value. However, in those situations, the variant name could simply be the boolean value. If we did that, removing feature_flag.evaluated.value may make sense. What do you think?

justinabrahms commented 2 years ago

@beeme1mr super happy with that.

dyladan commented 2 years ago

looks good. the only minor nitpick I have right now is that feature_flag.evaluated.variant implies that there may be other feature_flag.evaluated.* keys but I don't see any here. Maybe feature_flag.evaluated_variant?

beeme1mr commented 2 years ago

Thanks @dyladan, that was left over from when we had feature_flag.evaluated.variant and feature_flag.evaluated.value. I'll update it!

dyladan commented 2 years ago

I would even consider removing evaluated entirely as it seems to me it is implied the flag was evaluated, but maybe i'm missing some case where that isn't true.

beeme1mr commented 2 years ago

I've put together a quick flowchart to document how we could variants could be generated from values.

flowchart TD
    A[Start] --> B{Has Variant Name}
    B -->|Yes| C[Use It]
    C -->D[End]
    B -->|No| E{Value Type}
    E -->|Boolean| F[Stringify]
    F --> D
    E -->|Number| F
    %% E -->|String| H{Len>123}
    E -->|String| F
    G --> D
    E -->|Object| G[Hash]

Questions:

toddbaert commented 2 years ago

I think we might want to consider hashing strings. A few vendors specify strings in the 3000 byte range are possible as flag values - I think we should consider hashing them over a certain length.

dyladan commented 2 years ago

Is a byte array possible in any of these systems? Maybe good to include just for future-proofing

toddbaert commented 2 years ago

Is a byte array possible in any of these systems? Maybe good to include just for future-proofing

I haven't see byte array support from any existing vendor SDKs. A few mention strings are UTF-8 encoding explicitly, I suspect that's universally the case, though I'm not 100% sure. I suspect that most vendors would recommend base64 if there was a desire to store binary.

dyladan commented 2 years ago

Is a byte array possible in any of these systems? Maybe good to include just for future-proofing

I haven't see byte array support from any existing vendor SDKs. A few mention strings are UTF-8 encoding explicitly, I suspect that's universally the case, though I'm not 100% sure. I suspect that most vendors would recommend base64 if there was a desire to store binary.

Most vendors maybe but you don't want to exclude the possible future vendor who wants to support byte sequences

beeme1mr commented 1 year ago

Success!