-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: tracking #268
feat: tracking #268
Conversation
0a8a6ff
to
5efd0ed
Compare
Draft JS implementation available here: open-feature/js-sdk#1020 |
@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 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 Does anyone have points in favor of one or the other? |
Demo of some of these features: https://www.youtube.com/watch?v=Od-7Pjs2dKQ&t=436s |
The occurrence terminology threw me off initially. I understand the background for picking So I’m in favor of going with
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/ |
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. |
e81af61
to
a578c3f
Compare
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! 🙏 |
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 😅 . There's still a few cases of the word "occurrence" but I think they are simply descriptive, not params/types. |
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Co-authored-by: Michael Beemer <beeme1mr@users.noreply.github.com> Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Co-authored-by: Nicklas Lundin <nicklaslundin@gmail.com> Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Co-authored-by: Justin Abrahms <justin@abrah.ms> Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
04b1a1c
to
02164bc
Compare
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:
occurrenceKey
/context
/occurrenceDetails
.track
method should never block but instead queue any async work; we agreed there's no reason an application author would ever want to await a I/O associated with a singletrack
call; most providers will batch these, I assumevalue
a "first class" property of the otherwise free-formOccurrenceDetails
type, so it can be easily mapped into the equivalent field of various providers (something liketargetingKey
)