#4205: activate event emission #4240
Conversation
Because the predata can be inferred to be the opposite, this gives as much information and renders more cleanly in the UI
This is probably better left for a later PR
Sonatype Lift is retiringSonatype Lift will be retiring on Sep 12, 2023, with its analysis stopping on Aug 12, 2023. We understand that this news may come as a disappointment, and Sonatype is committed to helping you transition off it seamlessly. If you’d like to retain your data, please export your issues from the web console. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Ignored Deployments
|
thomasheartman
changed the base branch from
main
to
#4205/emit-potentially-stale-events
| const potentiallyStaleFeatures = | ||
| await this.featureToggleStore.updatePotentiallyStaleFeatures(); | ||
| if (this.flagResolver.isEnabled('emitPotentiallyStaleEvents')) { | ||
| if (potentiallyStaleFeatures.length) { |
There was a problem hiding this comment.
We need to check that there are actual events in here, because otherwise the batchStore methods logs a warning.
Arguably, the batchStore method should accept empty event lists and handle that gracefully. Maybe? I'd be happy to move the check into that store if you think that's sensible, but didn't want to touch more files than necessary.
This PR activates the event emission that was prepared for in #4239.
It emits events (behind a flag) when something is marked as potentially stale or the opposite.
It takes the features returned from the store and creates events out of them.
The events only contain data, no preData. This is because the preData can easily be inferred and because it gives a nicer event in the event log.
Here is an image of the difference. The top event uses only data, so it shows the name of the feature and the new potentiallyStale status. The bottom event uses both preData and data, so it only shows the new potentiallyStale status and not the feature name (unless you show the raw event):
Should not be merged before #4239. Merge that and then rebase this off main or cherry the commit.
Discussion
preDataShould we also use preData or is it enough to use only data? It seems unnecessary in this event, but I'm open to hearing your thoughts.
event author:
createdByI've set
unleash-systemas thecreatedByproperty on these events because they are generated by the system. I found the same string used some other places. However, it may be that there we want to use a different author.