feat: Add Metrics Hook

[askpt]

This PR

• This PR adds the metrics to the Feature Flag evaluation hook.

Related Issues

Fixes #113

[toddbaert]

@thisthat @beeme1mr adding you guys because you have a bit more OTel experience than me.

[thisthat]

We should look into why this pkg doesn't provide the attributes for FF: https://github.com/open-telemetry/opentelemetry-dotnet/tree/af83eb1043a34f82ba3bfe20c8b674ac295cfb37/src/OpenTelemetry.SemanticConventions

[toddbaert]

Not sure if they expect PRs for this or not, but we could open one.

[askpt]

It seems the library has not "updated" since 2022: https://www.nuget.org/packages/OpenTelemetry.SemanticConventions/1.0.0-rc9.9

Might need some help to reopen engagement there

[thisthat]

LGTM, I only left a minor suggestion :)
I don't know enough of C# to feel confident approving the PR tho, I'll delegate someone else for this :)

[toddbaert]

@beeme1mr is there a spec for these metrics somewhere? I know we have the trace semantics covered in OTel now - but I don't think there's anything specifically about metrics (correct me if I'm wrong)

LGTM, I only left a minor suggestion :) I don't know enough of C# to feel confident approving the PR tho, I'll delegate someone else for this :)

Yes, thanks @thisthat . I will review from a C# perspective, just glad to have you vet the OTel-specific stuff.

[toddbaert]

Really good work, much appreciated. I think my only concern is a lack of assertions on the attributes (if I missed that, please point me in the right direction).

[askpt]

Really good work, much appreciated. I think my only concern is a lack of assertions on the attributes (if I missed that, please point me in the right direction).

@toddbaert: after inspecting the Java library I noticed a few differences that I want to correct:

• Units are wrong. All are long and it should be another unit.
• Missing a clear table with Metrics exposed
• Doesn't support at the moment what they called Custom dimensions.
• Needs assertions at the tag list level. I am currently investigating this since I cannot find an easy way to assert the tag list.

[toddbaert]

• Doesn't support at the moment what they called Custom dimensions.

I noticed this, and it didn't immediately concern me because it's something that can be added later without breakage.

If you want, you can add it in this PR though. Otherwise I'll simply create a new issue.

• Needs assertions at the tag list level. I am currently investigating this since I cannot find an easy way to assert the tag list.

Here's how it was implemented in JS: https://github.com/open-feature/js-sdk-contrib/blob/1db63dca9a392c14f1574ec5ea8bda53f44123cd/libs/hooks/open-telemetry/src/lib/metrics/metrics-hook.spec.ts#L103-L112, if that's at all helpful!

[toddbaert]

@askpt I hope you don't mind I've added you here. This will add you to PRs for this component automatically, once you're an org member.

[askpt]

• Doesn't support at the moment what they called Custom dimensions.

I noticed this, and it didn't immediately concern me because it's something that can be added later without breakage.

If you want, you can add it in this PR though. Otherwise I'll simply create a new issue.

Please create a new issue. This PR is already big enough and it's not a breaking change as you mentioned.

• Needs assertions at the tag list level. I am currently investigating this since I cannot find an easy way to assert the tag list.

Here's how it was implemented in JS: https://github.com/open-feature/js-sdk-contrib/blob/1db63dca9a392c14f1574ec5ea8bda53f44123cd/libs/hooks/open-telemetry/src/lib/metrics/metrics-hook.spec.ts#L103-L112, if that's at all helpful!

I am trying to find a way to do it but it doesn't seem possible. The counter tags are private in the meter class tagsToMetricPointIndexDictionary. This is all I can find:

[toddbaert]

• Doesn't support at the moment what they called Custom dimensions.

I noticed this, and it didn't immediately concern me because it's something that can be added later without breakage.
If you want, you can add it in this PR though. Otherwise I'll simply create a new issue.

Please create a new issue. This PR is already big enough and it's not a breaking change as you mentioned.

Done.

• Needs assertions at the tag list level. I am currently investigating this since I cannot find an easy way to assert the tag list.

Here's how it was implemented in JS: https://github.com/open-feature/js-sdk-contrib/blob/1db63dca9a392c14f1574ec5ea8bda53f44123cd/libs/hooks/open-telemetry/src/lib/metrics/metrics-hook.spec.ts#L103-L112, if that's at all helpful!

I am trying to find a way to do it but it doesn't seem possible. The counter tags are private in the meter class tagsToMetricPointIndexDictionary. This is all I can find:

OK in that case I'm tempted to call the testing good enough, given the limitations.

[toddbaert]

Approved in light of this comment.

[toddbaert]

I'll merge this EOD unless I hear objections.