Skip to content

Conversation

@kylejuliandev
Copy link
Contributor

@kylejuliandev kylejuliandev commented Jul 5, 2025

This PR

Adds new MetricsHookOptions and MetricsHookOptionsBuilder to optionally configure custom attributes that can be tagged on the feature_flag.evaluation_success_total metric. Example usage:

var options = MetricsHookOptions.CreateBuilder()
    .WithCustomDimension("custom_dimension_key", "custom_dimension_value")
    .WithFlagEvaluationMetadata("boolean", s => s.GetBool("boolean"))
    .Build();

OpenFeature.Api.Instance.AddHooks(new MetricsHook(options));

Screenshot below shows the AspNetCore sample application tagging the feature_flag.evaluation_success_total counter with the specified dimensions.

image

Related Issues

Fixes #509
Fixes #514

Notes

Follow-up Tasks

How to test

Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
@kylejuliandev kylejuliandev requested a review from a team as a code owner July 5, 2025 10:13
@codecov
Copy link

codecov bot commented Jul 5, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.73%. Comparing base (929fa74) to head (a278501).
Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #512      +/-   ##
==========================================
+ Coverage   87.35%   87.73%   +0.38%     
==========================================
  Files          47       48       +1     
  Lines        1827     1876      +49     
  Branches      190      196       +6     
==========================================
+ Hits         1596     1646      +50     
  Misses        188      188              
+ Partials       43       42       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@askpt askpt left a comment

Choose a reason for hiding this comment

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

LGTM. Just a small note on a future improvements to the unit tests.

Copy link
Member

@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.

The feature looks great but I had a question about the docs. I also wonder if we should consider supporting something similar for traces.

@askpt
Copy link
Member

askpt commented Jul 7, 2025

The feature looks great but I had a question about the docs. I also wonder if we should consider supporting something similar for traces.

@beeme1mr That tracker idea might be something useful. Should we create an issue to track it if makes sense?

Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
@askpt askpt enabled auto-merge July 14, 2025 17:55
github-merge-queue bot pushed a commit that referenced this pull request Jul 14, 2025
…ce (#513)

<!-- Please use this template for your pull request. -->
<!-- Please use the sections that you need and delete other sections -->

## This PR
<!-- add the description of the PR here -->

In #512 I needed to tweak the sample AspNetCore application to pass Hook
options to the MetricsHook. I noticed that in order to pass the options
I needed to use the delegate based approach, while discarding the
`serviceProvider`, like:

```csharp
featureBuilder.AddHostedFeatureLifecycle()
        .AddHook(_ => new MetricsHook(metricsHookOptions))
```

It would be simpler and easier for devs to interact with a third
overload that allows them to pass an instance of the hook that they want
to interact with, like so:

```csharp
featureBuilder.AddHostedFeatureLifecycle()
    .AddHook(new MetricsHook())
```

### Related Issues
<!-- add here the GitHub issue that this PR resolves if applicable -->

### Notes
<!-- any additional notes for this PR -->

### Follow-up Tasks
<!-- anything that is related to this PR but not done here should be
noted under this section -->
<!-- if there is a need for a new issue, please link it here -->

### How to test
<!-- if applicable, add testing instructions under this section -->

---------

Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
Copy link

@weyert weyert left a comment

Choose a reason for hiding this comment

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

LGTM

@askpt askpt added this pull request to the merge queue Jul 19, 2025
Merged via the queue into open-feature:main with commit 8c05d1d Jul 19, 2025
18 checks passed
@kylejuliandev kylejuliandev deleted the feat/add-metric-hook-custom-attributes branch July 19, 2025 16:28
@askpt askpt mentioned this pull request Jul 21, 2025
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.

Refactor MetricsHook unit tests to use new MetricCollector pattern [OTel Hook] Add custom-dimensions support to Metrics Hook

5 participants