feat(flagd): Opentelemetry feature for metrics/spans#92
feat(flagd): Opentelemetry feature for metrics/spans#92
Conversation
Signed-off-by: Eren Atas <eren_atas@hotmail.com>
Summary of ChangesHello @erenatas, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Signed-off-by: Eren Atas <eren_atas@hotmail.com>
33baa26 to
98d6905
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces a significant new feature: OpenTelemetry support for metrics and tracing across all evaluation modes. The implementation is comprehensive, including a new otel module with well-structured submodules for gRPC, HTTP, metrics, and spans, along with extensive tests using a fake collector.
Beyond the new feature, this PR includes several impressive refactorings that improve the provider's robustness and performance:
- Error handling has been standardized to a custom
FlagdErrorenum, replacinganyhow. - The file-based resolver now uses the
notifycrate for efficient, event-driven file watching instead of polling. - The gRPC stream connector has been enhanced with connection reuse, better keep-alive handling, and more resilient retry logic.
- The in-process targeting engine now correctly serializes nested evaluation context structures, which is a critical improvement.
Overall, this is an excellent set of changes that significantly enhances the flagd provider. My only minor suggestion is to avoid a fixed sleep in one of the new examples to prevent potential flakiness.
I am having trouble creating individual review comments. Click here to see my feedback.
crates/flagd/examples/common/mod.rs (39)
The use of a fixed-duration sleep can lead to flaky examples, as the required initialization time for flagd might vary depending on the environment. The container is already configured with with_wait_for to wait for a specific log message. If additional initialization is needed after the port is available, a more robust approach would be to poll a health endpoint or wait for another specific condition rather than using a fixed sleep. While acceptable for an example, avoiding arbitrary sleeps is a good practice to ensure reliability.
crates/flagd/src/resolver/in_process/targeting/mod.rs (129-137)
The previous implementation for handling EvaluationContextFieldValue::Struct used format!("{:?}", s), which would not correctly serialize the struct for the JSONLogic engine. This change to properly downcast to open_feature::StructValue and recursively serialize it is a critical fix that enables complex, nested evaluation contexts to work correctly with in-process targeting rules. This is an excellent improvement.
beeme1mr
left a comment
There was a problem hiding this comment.
Hey @erenatas, thanks for the PR. Would it be possible to publish as a separate hook so that any provider could use it? Also, do the OTel Rust SDK support span events? That's the recommended way to capture feature flag evaluations (at least for in-process evals) since a span tends to clutter most tracing UIs.
This change is on top of #91, temporarily changing target branch to show diff correctly
Add optional
otelcargo feature that enables OpenTelemetry tracingand metrics for all evaluation modes (RPC, REST, in-process, file).
Tracing:
Metrics:
Attributes include: