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

feat: Initial Proposal for Diagnostic Feature Codes in OpenFeature #3

Conversation

arttonoyan
Copy link
Owner

This PR introduces a new structure for Diagnostic Feature Codes in OpenFeature, aimed at facilitating consistent tracking and conditional enabling of experimental features. The purpose of this addition is to propose a standardized code format for community discussion and feedback.

Summary of Changes:

  • Added FeatureCodes class under OpenFeature.Diagnostics namespace to define identifiers for experimental features.
  • Implemented a structured code format for diagnostics: [Prefix][Domain][UniqueNumber], exemplified by OFDI001, where:
    • OF - Represents the OpenFeature library.
    • DI - Indicates the specific feature domain (e.g., Dependency Injection).
    • 001 - Unique identifier for each feature in the domain.

Objectives:

This initial proposal serves as a proof of concept and a foundation for potential Diagnostics Specifications in OpenFeature. By aligning with best practices seen in OpenTelemetry and EF Core diagnostics, we can establish a framework that:

  1. Encourages a consistent and organized approach to managing experimental features.
  2. Allows developers to reference and suppress diagnostic codes where necessary.
  3. Simplifies tracking, documentation, and usage of evolving features.

Community Discussion:

I encourage the community to review this initial approach and provide feedback on:

  • The structured format of diagnostic codes.
  • How this approach aligns with OpenFeature’s goals and usability.
  • Suggestions for improvements or additional elements in a future Diagnostics Specification for OpenFeature.

Note

I was limited in time to explore all potential aspects of this approach, but I have attempted to standardize the code format as a foundation for future enhancement. This is an initial investigation, and I hope it can serve as a basis for discussion and refinement. Please consider this a starting point, and I look forward to your insights on how we can expand or improve upon this concept.

This PR is an exploration and is open for community input. Please share any thoughts, concerns, or additional ideas for implementation.

Copy link

@beeme1mr beeme1mr left a comment

Choose a reason for hiding this comment

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

Looks like a good approach to me. 👍

/// <strong>Basic Information</strong><br/>
/// These identifiers conform to OpenFeature’s Diagnostics Specifications, allowing developers to recognize
/// and manage experimental features effectively. For more details, refer to the
/// <a href="https://github.com/open-feature/dotnet-sdk/docs/diagnostics/README.md">Diagnostics Specifications</a>.
Copy link

Choose a reason for hiding this comment

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

Reminder to add this readme or a new section could be added to the root readme.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Absolutely, I’ve placed it here as an example for now. If the team decides it’s a good idea and we move forward with documenting it, I’ll update the path to the correct location (e.g., adding it to the root README or a new section). Otherwise, I’ll simply remove it. For now, I’m waiting on some feedback and approval to ensure my changes align with what the team wanted, so I can finalize everything accordingly.

And thanks for the quick response!

@askpt
Copy link

askpt commented Nov 6, 2024

I was wondering in the feature codes if we could use the GitHub issue ID as a link to the Feature code. For example, for the DI, it would be OF264 where the 264 links to open-feature#264

@arttonoyan
Copy link
Owner Author

I was wondering in the feature codes if we could use the GitHub issue ID as a link to the Feature code. For example, for the DI, it would be OF264 where the 264 links to open-feature#264

@askpt it’s an interesting idea! However, I think it’s best to avoid adding dependencies between feature codes and GitHub issue IDs, as they serve different purposes. Instead, we could use labels to map PRs to feature codes. This would keep feature codes independent and allow flexibility in linking relevant PRs without tightly coupling them to specific GitHub issues.

For example, if we have a feature code OFDI001 for a Dependency Injection enhancement:

  • We could label the PR with feature-code: OFDI001 along with Dependency Injection and experimental.

This way, we maintain a structured system for feature codes while using labels to create a flexible, searchable link to PRs and discussions.

@askpt
Copy link

askpt commented Nov 6, 2024

I like this work here. My only concern is that it will be visible only to developers on .NET 8+. I am searching if we can find a way to use this approach to alert users not on this SDK version. Other than that, it looks great @arttonoyan thanks!

@toddbaert
Copy link

toddbaert commented Nov 6, 2024

Big fan of this approach.

I like this work here. My only concern is that it will be visible only to developers on .NET 8+. I am searching if we can find a way to use this approach to alert users not on this SDK version. Other than that, it looks great @arttonoyan thanks!

TBH I think support 8+ only is acceptable for now. I think our project is relatively new, so I expect that the majority of our users would have still-actively-maintained codebases, which at least soon, should be on .NET 8+... maybe that's naive.

@arttonoyan
Copy link
Owner Author

arttonoyan commented Nov 7, 2024

@toddbaert could you please confirm these changes? If they’re good to go, I’ll proceed with the merge (and I’ll adjust the link section as needed).

@beeme1mr, @toddbaert, @askpt Since the code approach has already been reviewed, I’d like to hear your thoughts on it. Do you think it’s worth documenting, and if so, where would be the best place? If there’s still room for improvement, please share any suggestions for the next steps.

I’d like to wrap this up soon, as I'm already working on other improvements that some teams in our company need - Of course, before moving forward, I’ll first share the ideas I’d like to implement with you and explain why they’re needed.

@beeme1mr
Copy link

beeme1mr commented Nov 7, 2024

I think it's worth briefly mentioning this in the contributing docs. I also agree that we should try and get it in ASAP. The change looks great and will be clearly marked as experimental.

@toddbaert
Copy link

toddbaert commented Nov 8, 2024

@arttonoyan I can't add myself as a reviewer, it seems, but I agree with everything here.

@arttonoyan arttonoyan merged commit 6f8c5be into add-dependency-injection Nov 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants