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

Add feature flagging semantic conventions #2529

Merged
merged 29 commits into from
Dec 1, 2022

Conversation

beeme1mr
Copy link
Contributor

@beeme1mr beeme1mr commented May 10, 2022

Resolves #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

@beeme1mr beeme1mr requested review from a team May 10, 2022 21:33
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented May 10, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: beeme1mr / name: Michael Beemer (f1aa873)

@tigrannajaryan
Copy link
Member

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.

@tigrannajaryan
Copy link
Member

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.

@beeme1mr
Copy link
Contributor Author

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
Copy link
Contributor Author

beeme1mr commented May 11, 2022

I just noticed that you clarified the process a PR. I'll open an issue tomorrow. Thanks!

@reyang
Copy link
Member

reyang commented May 11, 2022

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)?

@dyladan
Copy link
Member

dyladan commented May 11, 2022

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.

@tigrannajaryan
Copy link
Member

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.

@reyang
Copy link
Member

reyang commented May 12, 2022

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?

@dyladan
Copy link
Member

dyladan commented May 12, 2022

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.

@reyang reyang added the area:semantic-conventions Related to semantic conventions label May 13, 2022
@github-actions
Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label May 21, 2022
@dyladan
Copy link
Member

dyladan commented May 24, 2022

This is not stale

@beeme1mr beeme1mr marked this pull request as draft May 24, 2022 19:27
@beeme1mr
Copy link
Contributor Author

Marking this as a draft until #2532 is resolved.

@github-actions github-actions bot removed the Stale label May 25, 2022
@github-actions
Copy link

github-actions bot commented Jun 1, 2022

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Jun 1, 2022
@dyladan
Copy link
Member

dyladan commented Jun 1, 2022

Not stale

@github-actions github-actions bot removed the Stale label Jun 2, 2022
@github-actions
Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Jun 10, 2022
@beeme1mr
Copy link
Contributor Author

Still waiting for an official response on #2532

@github-actions github-actions bot removed the Stale label Jun 16, 2022
@dyladan dyladan requested a review from a team November 21, 2022 20:46
@dyladan
Copy link
Member

dyladan commented Nov 21, 2022

Updated to add a log version

@beeme1mr
Copy link
Contributor Author

Hi @tigrannajaryan and @jsuereth, does the recent changes made by @dyladan address your concerns?

Copy link
Contributor

@jsuereth jsuereth left a comment

Choose a reason for hiding this comment

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

This answers my main concerns on log/trace usage. Just a minor nit / visibility concern left.

@dyladan
Copy link
Member

dyladan commented Nov 30, 2022

Can someone with permission please run the link check again? It failed on jsonlines.org which is not a dead link

@reyang reyang closed this Nov 30, 2022
@reyang reyang reopened this Nov 30, 2022
@dyladan
Copy link
Member

dyladan commented Nov 30, 2022

Link check is passing locally and appears to be on a file we haven't touched

@dyladan
Copy link
Member

dyladan commented Dec 1, 2022

@tigrannajaryan @SergeyKanzhelev I think all the feedback has been addressed and there are 5 green approvals. I think this can probably be merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:semantic-conventions Related to semantic conventions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Semantic Convention for Feature Flagging