opentelemetry-specification
opentelemetry-specification copied to clipboard
Add feature flagging semantic conventions
Resolves https://github.com/open-telemetry/opentelemetry-specification/issues/2532
Changes
Added semantic conventions for representing feature flags as spans. This can be used to determine the impact a feature has on a request.
Related issues
- https://github.com/open-telemetry/opentelemetry-specification/issues/2532
- https://github.com/open-feature/spec/issues/30
The committers listed above are authorized under a signed CLA.
- :white_check_mark: login: beeme1mr / name: Michael Beemer (f1aa8732f2059f6051fc7ce0ed542292a11c2602)
This needs an issue in this repository (a reference to an issue in another repository is useful but not sufficient) and then follow the issue workflow. Ideally this should have happened before the PR is created so that we know we want it in the OpenTelemetry.
This needs an issue in this repository (a reference to an issue in another repository is useful but not sufficient) and then follow the issue workflow. Ideally this should have happened before the PR is created so that we know we want it in the OpenTelemetry.
You can wait a bit and see if there are approvers commenting on this PR who think the PR is a good idea then we can move forward without an issue.
Hi @tigrannajaryan, sorry for not following protocol. I'm happy to close this PR until it has gone through the proper channels. However, I'm not sure what the proper channel is. The contributing doc that you sent says "significate changes" require an OTEP and that small changes can be made directly via a PR. This change seems to fall somewhere in-between those two extremes.
I just noticed that you clarified the process a PR. I'll open an issue tomorrow. Thanks!
May I get a short summary regarding why feature-flagging needs to be part of semantic conventions? Is there an example (I read through the linked issue and couldn't find a clear explanation)?
May I get a short summary regarding why feature-flagging needs to be part of semantic conventions? Is there an example (I read through the linked issue and couldn't find a clear explanation)?
If feature flag keys and values are consistently reported, it will enable splitting traces and metrics by these values for usecases like A/B testing, feature rollouts, and similar. In order for this to work it must be consistent across all microservices, potentially in multiple languages and/or using multiple feature flag services.
Hi @tigrannajaryan, sorry for not following protocol. I'm happy to close this PR until it has gone through the proper channels. However, I'm not sure what the proper channel is. The contributing doc that you sent says "significate changes" require an OTEP and that small changes can be made directly via a PR. This change seems to fall somewhere in-between those two extremes.
@beeme1mr no problem, we need the contributing documentation to be more clear. I think it is fine to keep this PR open.
If feature flag keys and values are consistently reported, it will enable splitting traces and metrics by these values for usecases like A/B testing, feature rollouts, and similar. In order for this to work it must be consistent across all microservices, potentially in multiple languages and/or using multiple feature flag services.
If the user has System X calling into Cloud Provider Y which in turn calls back to System Z, and they use different A/B testing / feature flag system, what would be the expectation? Or this PR is specific about open-features
and it's compatible systems, not about other systems?
If feature flag keys and values are consistently reported, it will enable splitting traces and metrics by these values for usecases like A/B testing, feature rollouts, and similar. In order for this to work it must be consistent across all microservices, potentially in multiple languages and/or using multiple feature flag services.
If the user has System X calling into Cloud Provider Y which in turn calls back to System Z, and they use different A/B testing / feature flag system, what would be the expectation? Or this PR is specific about
open-features
and it's compatible systems, not about other systems?
They would just report span attributes consistent with the feature flag system they're using. The attributes are general enough to be used with any feature flag system. I'm not sure I see what the problem is.
This PR was marked stale due to lack of activity. It will be closed in 7 days.
This is not stale
Marking this as a draft until #2532 is resolved.
This PR was marked stale due to lack of activity. It will be closed in 7 days.
Not stale
This PR was marked stale due to lack of activity. It will be closed in 7 days.
Still waiting for an official response on #2532
This PR was marked stale due to lack of activity. It will be closed in 7 days.
I removed the Stale label to give more time for people to express their support at https://github.com/open-telemetry/opentelemetry-specification/issues/2532
@beeme1mr
One thing that may be worth adding is a evaluation_reason
or something similar. This would identify why a particular flag value was returned (some targeting rule was matched, a random split is configured, or a static flag value was assigned, for instance):
- id: evaluation_reason
type: string
requirement_level: recommended
brief: A string indicating why a particular variant was selected.
examples: ["STATIC", "TARGETING_MATCH". "PSEUDORANDOM_SPLIT"]
Such a concept exists in OpenFeature as well as some vendors, for example: https://docs.launchdarkly.com/sdk/concepts/evaluation-reasons
@beeme1mr
One thing that may be worth adding is a
evaluation_reason
or something similar. This would identify why a particular flag value was returned (some targeting rule was matched, a random split is configured, or a static flag value was assigned, for instance):- id: evaluation_reason type: string requirement_level: recommended brief: A string indicating why a particular variant was selected. examples: ["STATIC", "TARGETING_MATCH". "PSEUDORANDOM_SPLIT"]
Such a concept exists in OpenFeature as well as some vendors, for example: docs.launchdarkly.com/sdk/concepts/evaluation-reasons
I see a couple problems with this (although none that can't be overcome).
- I can't see what obvious value that provides. Knowing that a flag is
true
because it targets specific users is possible from just the key and the data stored in your feature flag system. - I think this might be tough to implement in a way that makes sense across all tools/vendors. In order for the reason to be meaningful, each reason would have to have a definition and all vendors would have to agree on them. There is also a lot of potential crossover between different reasons. For example, if I split 50% of my traffic based on the hash of the user's email, is that
TARGETING_MATCH
or is thatPSEUDORANDOM_SPLIT
?
I think in this case I would advocate to get the basic functionality already in the PR merged, then see what users ask for when they start actually consuming telemetry data.
- I can't see what obvious value that provides. Knowing that a flag is
true
because it targets specific users is possible from just the key and the data stored in your feature flag system.
This is true, but it might be tedious to cross reference such configuration when viewing telemetry data. The reason
can help make it obvious that some specific targeting occurred "at a glance", perhaps in a situation when a static value was expected (maybe somebody forgot to remove some specific targeting that was added for debug purposes).
Vendors thought this information was important enough to include it in their API, so I think it could be important for the semantic conventions.
- I think this might be tough to implement in a way that makes sense across all tools/vendors. In order for the reason to be meaningful, each reason would have to have a definition and all vendors would have to agree on them. There is also a lot of potential crossover between different reasons. For example, if I split 50% of my traffic based on the hash of the user's email, is that
TARGETING_MATCH
or is thatPSEUDORANDOM_SPLIT
?
I'm not sure that there needs to be a precise consensus on the possible values of this field across all providers for it to be valuable, but I see your point. Perhaps a lack of consistency here is a dealbreaker :shrug:
I think in this case I would advocate to get the basic functionality already in the PR merged, then see what users ask for when they start actually consuming telemetry data.
Fine with me!
- I can't see what obvious value that provides. Knowing that a flag is
true
because it targets specific users is possible from just the key and the data stored in your feature flag system.This is true, but it might be tedious to cross reference such configuration when viewing telemetry data. The
reason
can help make it obvious that some specific targeting occurred "at a glance", perhaps in a situation when a static value was expected (maybe somebody forgot to remove some specific targeting that was added for debug purposes).
¯\(ツ)/¯ the value there seems like a stretch to me, but maybe I'm wrong.
Vendors thought this information was important enough to include it in their API, so I think it could be important for the semantic conventions.
This is definitely not reason enough on its own. There are all sorts of things available from host/process/library APIs that we don't include in semconv.
Like I said. I think all of these things can be overcome, but it is a non-breaking addition and can always be included later if it becomes important.
Hi @tigrannajaryan and @SergeyKanzhelev, do you have any additional questions or concerns I can address?
This PR was marked stale due to lack of activity. It will be closed in 7 days.
Removing the Stale label. It would be good to see the questions I raised here answered by this PR.
This PR was marked stale due to lack of activity. It will be closed in 7 days.
I am still not convinced the feature-flag data in the proposed form is useful, however I won't block the PR and if sufficient number of other approvers find it useful it can get merged.
Would you envisage feature flag attributes appearing as a "resource" attribute rather than just a tracing span attribute?
We sometimes run experiments with the same source code run side-by-side in 2 deployments deployment (with randomly distributed A/B style user traffic), but often with different compilation options (e.g. -O3 vs. -O2), or perhaps running on JVM 11 vs. JVM 14. Usually this is for internal regression analysis/comparison of performance.
I could imagine creating a feature_flag.flag_key=jvm
, feature_flag.evaluated_variant=jdk14
for such a situation and adding them to the resource, but I don't know if this is what is intended.
For JDK, it seems like process.runtime.name
and process.runtime.version
are probably sufficient , but there are other situations where we may fix the value of a feature_flag for a particular deployment, so that everything generated by that entity will have that feature_flag set to the fixed value, while our comparison deployment will have the feature flag set to off.
Personally for our case, I thought something like a service.variant
(or perhaps just a suffixed service.version
) would be appropriate given that service.*
is already associated with Resources, but this PR caught my attention since it has some similarity in the semantics.
Hi @evantorrie, you could certainly use this proposal for analyzing A/B tests. For example, you could select what Fibonacci algorithm to run using a feature flag. The results could be added to the trace as a dedicated span. In many tools, you can search for specific span name and value to see the full trace. This would allow you to compare the various states to see how the flag impacted the request.
Would you envisage feature flag attributes appearing as a "resource" attribute rather than just a tracing span attribute?
From my understanding, resource attributes are meant to be set when the system starts and should be immutable. A feature flag can be dynamically changed during runtime. The value can also be different based on evaluation context (i.e. enabled for a specific user).