Skip to content
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

FFM-11212 Metrics enhancements #150

Merged
merged 44 commits into from
Apr 19, 2024
Merged

FFM-11212 Metrics enhancements #150

merged 44 commits into from
Apr 19, 2024

Conversation

erdirowlands
Copy link
Contributor

What

  • Splits evaluation and target metrics into their own caches:

    • so that we can track target metrics correctly. Due to the fact we use the globalTarget , this meant that we would miss targets if the evaluation didn't change.
    • Implements limits: Evaluation metrics to 10K unique evaluations--where a unique evaluation is flag + variation--this allows for evaluation for 10k flags with 5 variations each
    • Target metrics for 100K unique targets
  • Implement seen targets:

    • ensures that the metrics service only gets sent target metrics for a target once, for the lifetime of the SDK instance
    • ensures that the SDK processes truly unique targets fairly, and that old targets don't occupy the 100K limit set per interval

Testing

  • Metrics accuracy:
    • New unit tests
    • TestGrid metrics tests
    • Manual sample app
    • Non-anonymous targets appear in UI
    • Anonymous targets don't appear in UI
  • SeenTargets:
    • New unit tests and manual testing
  • Limits:
    • Manual testing: Sent 101K unique targets, only 100K get processed

analyticsChan chan analyticsEvent
evaluationsAnalyticsMx *sync.Mutex
targetAnalyticsMx *sync.Mutex
seenTargetsMx *sync.RWMutex
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opted for a RWMutex here because the seen targets cache should mostly be reads once it's filled up. It's possible a user could send all unique targets, but it is more likely in my estimation that targets will be reused.

seenTargets map[string]bool
timeout time.Duration
logger logger.Logger
metricsClient metricsclient.ClientWithResponsesInterface
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a pointer to an interface which I removed as it prevented me from using my own types for testing purposes.

@erdirowlands erdirowlands merged commit 5d1c3b7 into main Apr 19, 2024
3 checks passed
@erdirowlands erdirowlands deleted the FFM-11212 branch April 19, 2024 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants