[FEATURE] Refactor telemetry package

[sahidvelji]

Requirements

I would like to improve the telemetry package:

• Omit the value from the returned attributes. The value is of type any and this results in the loss of type safety (use of map[string]any). Instead, by omitting the value, the attributes map can be of type map[string]string, which is much easier to work with. The value is otherwise very easy to obtain via details.Value anyways.
• The "Name" of the evaluation event should not be returned. Instead, since that is already a const in the package, users can just use that const wherever it is needed. Ideally, this event name const feature_flag.evaluation should eventually make its way into the OTel semconv (as a const). Currently it is in the spec but not in the semconv package.
• Refactor the CreateEvaluationEvent function to make it work for ALL stages of a hook, rather than just after and finally. To do that, we could modify the details openfeature.InterfaceEvaluationDetails parameter to be a pointer so that a nil check can be performed before adding additional attributes derived from the details. This allows the package to be a bit more useful. This is not a huge deal though, given that there are only two attributes in the hookContext.

I think the goal should be for this package to be used in both the existing logging hook and the OTel hook in go-sdk-contrib.

Related issue: #393

[beeme1mr]

Hey @sahidvelji, I think the only reason map[string]any is used is because the value isn't always a string. If there's a clean way to support value without losing type safety, I'm 100% in favor of that.

@dyladan, do you know if feature_flag.evaluation can be added as a const to the semcon package?

Why would you like to run CreateEvaluationEvent in other hook stages? I'm not against the idea; I'm just wondering.

[sahidvelji]

I think the only reason map[string]any is used is because the value isn't always a string. If there's a clean way to support value without losing type safety, I'm 100% in favor of that.

What I am proposing is omitting value from the map entirely and have users get that value themselves. It's a minor inconvenience for users. The alternative of including it in the map is far worse than a minor inconvenience because of the loss of type safety IMO.

@dyladan, do you know if feature_flag.evaluation can be added as a const to the semcon package?

It seems like the semconv package is mostly meant for attributes (key-value pairs) rather than consts, but I'm not certain. I'll let @dyladan confirm, but it would be great to include that given that it's part of the spec.

Why would you like to run CreateEvaluationEvent in other hook stages? I'm not against the idea; I'm just wondering.

I figured making the package be more general would be useful for the metrics/traces/logging hooks. But on second thought, I think it's fine as is.

[beeme1mr]

Ah, I understand. Are you recommending that we update the existing function or add another one? The current function is used in the OTel hook, so we'd need to be careful not to break that hook.

[sahidvelji]

Update the existing function. I don't see the telemetry package being used in the OTel hooks actually https://github.com/open-feature/go-sdk-contrib/tree/main/hooks/open-telemetry/pkg

[beeme1mr]

Okay, I must be misremembering. I'm reluctant to make a breaking change but I'd defer to you if you think it's worth it. It's a bummer that Go's type system is so inflexible.

[sahidvelji]

I say we go ahead with it. I don't imagine many people are actually using the telemetry package at all given that it was just released a few weeks ago on June 2nd. It would certainly provide a more ergonomic user experience. Alternatively we could scope this for v2 of the SDK if we really don't want a breaking change here.

[beeme1mr]

I don't think we need to wait for v2 since we never announced its availability and it isn't used in the hook (yet).