open-feature / spec

OpenFeature specification
https://openfeature.dev
Apache License 2.0
779 stars 36 forks source link

feat: tracking #268

Closed toddbaert closed 1 month ago

toddbaert commented 3 months ago

Adds tracking


This PR is a first pass at a relatively simple tracking API. The main purpose is to close what is more or less the last gap between the OpenFeature API and that of most SDKs (many SDKs which we currently support have some kind of simple "track" method analogous to what's proposed here). Preceding discussion can be found here.

Some notable choices:

kozikow commented 3 months ago

It should probably be included in another proposal, but I think it would be also useful to include a concept of "occurence aggregation". At it's core it would be like "list of SQL queries on occurences".

E.g. see following example - this is a fake scenario built on flagd + openfeature - here track occurence is upvote or downvote. Explore "metrics" by flag, variant: image

Here "aggregation" is "Sentiment Score": AVG(IF(occurence_key="feedback_up", 1, IF(occurence_key="feedback_down", -1, 0))) and other similar type of metrics.

Then various dashboarding providers can out of the box set up experiment evaluation dashboard. Here Bigquery + Looker Studio is used, but the same flow would be possible with open source Metabase for example.

nicklasl commented 3 months ago

It should probably be included in another proposal, but I think it would be also useful to include a concept of "tracking aggregation". At it's core it would be like "list of SQL queries on tracking events". ... Then various dashboarding providers can out of the box set up experiment evaluation dashboard. Here Bigquery + Looker Studio is used, but the same flow would be possible with open source Metabase for example.

While I appreciate the idea of incorporating "tracking aggregation" as a way to streamline dashboard creation, I believe it's crucial to keep the specifications clean and neutral regarding the underlying technology for data storage, combination, and aggregation. This approach ensures that our specifications remain versatile and adaptable, allowing various teams/vendors to implement them using the tools and technologies that best fit their needs and existing infrastructure.

For example, there should be nothing stopping me from building a tracking provider that just outputs to a file if that's what my business requires.

However, I am open to have some type of documentation of best practices or examples of use and maybe this is where such proposal belong?

toddbaert commented 2 months ago

Draft JS implementation available here: https://github.com/open-feature/js-sdk/pull/1020

toddbaert commented 2 months ago

@moredip suggests we perhaps use something other than "occurrence" (see comment here in draft JS implementation).

I went with "occurrence" to reduce collision with our existing event concepts (for example, we already have event and eventDetails objects), but we can disambiguate these other ways:

Currently I've proposed:

void track(String occurrenceKey, EvaluationContext context, OccurrenceDetails details): void;

but an obvious alternative would be:

void track(String trackingEventKey, EvaluationContext context, TrackingEventDetails details): void;

I'm also not sure if trackingEventId or trackingEventKey is better; we use key for flags themselves, but a key to me suggests a string index a specific set (perfect for flags in flag-sets) whereas an id might be a "softer" reference, such as a UUID.

Does anyone have points in favor of one or the other?

toddbaert commented 2 months ago

Demo of some of these features: https://www.youtube.com/watch?v=Od-7Pjs2dKQ&t=436s

roncohen commented 2 months ago

Does anyone have points in favor of one or the other?

The occurrence terminology threw me off initially. I understand the background for picking occurrence over event/trackingEvent, but I feel that in the context of the track call, the confusion should be manageable and the advantage of aligning on known terminology is substantial.

So I’m in favor of going with trackingEvent, trackEvent or simply event (and trackingEventKey, trackEventKey or eventKey respectively)

I'm also not sure if trackingEventId or trackingEventKey is better

Agreed that “key” is better here. Some platforms give specific events UUIDs for deduplication purposes etc which is not what we’re talking about here. I think “name” could also be a contender. Segment refers to this as the name (e.g. “event name”): https://segment.com/docs/connections/spec/track/

roncohen commented 2 months ago

In the interest of moving this forward, I'd go for:

void track(String trackingEventName, EvaluationContext context, TrackingEventDetails details): void;
toddbaert commented 2 months ago

In the interest of moving this forward, I'd go for:

void track(String trackingEventName, EvaluationContext context, TrackingEventDetails details): void;

I like this and based on slack we have a consensus. Will update.

toddbaert commented 2 months ago

I've made updates based on the most recent feedback, and rebased on main. As usual with spec PRs, we want a wide consensus; so far I think I've seen that represented in this PR and in various meetings and communications in Slack.

I will leave this PR open for additional comments, but I plan to merge it early next week. If you have comments or concerns, please voice them here before then! :pray:

roncohen commented 2 months ago

Nice @toddbaert. Looks like there's still some "occurrence" terminology left in this PR.

toddbaert commented 2 months ago

Nice @toddbaert. Looks like there's still some "occurrence" terminology left in this PR.

Nice catch, @roncohen . I think I got them all this time :sweat_smile: . There's still a few cases of the word "occurrence" but I think they are simply descriptive, not params/types.