This PR supersedes #57 and is based on input from @rbtcollins and @sighphyre both in that PR and in a separate conversation in Slack.
Description
Variant metrics should be counted whenever get_variant is called:
if a feature doesn't have variants, register the disabled variant
if a feature is disabled, register the disabled variant
if a feature doesn't exist, it should still register the disabled variant.
otherwise, register the variant that was returned
This PR implements just the counting of the metrics. It does not implement serialization and submission to Unleash. I'd like to handle that in a follow-up PR after we agree on the implementation of the counting.
Improvements from #57
This PR has a smaller impact than #57 in two significant ways:
It does not introduce any new dependencies
It does not negatively affect benchmarks (ran locally)
Implementation
The disabled variant is counted on the feature level (CachedFeature), while each actual variant is counted on the individual variant (CachedVariant).
This PR introduces a new CachedVariant struct. That struct wraps all the data that exists in api::Variant and adds a new count property.
Additionally, I've introduced a variants_metrics method for the CachedFeature struct serializes its variant metrics into hash map.
Discussion points
Manual counting of variants
I've left the task of counting variants as a manual job, keeping in line with how yes and no are counted. We could add an increment_count method to the CachedVariant struct, but I don't know if it's worth it.
Benches
Running the benches locally, I see no significant changes between the main branch and this. All differences appear to be within the noise threshold.
The one benchmark that is the most interesting is probably the "batch/parallel unknown-features(str)", which took a pretty significant hit in #57.
Running it with the latest changes in this branch (against main), actually resulted in improved performance. This is probably noise, but it at least seems to indicate that this is fairly safe.
This PR supersedes #57 and is based on input from @rbtcollins and @sighphyre both in that PR and in a separate conversation in Slack.
Description
Variant metrics should be counted whenever
get_variant
is called:disabled
variantdisabled
variantdisabled
variant.This PR implements just the counting of the metrics. It does not implement serialization and submission to Unleash. I'd like to handle that in a follow-up PR after we agree on the implementation of the counting.
Improvements from #57
This PR has a smaller impact than #57 in two significant ways:
Implementation
The disabled variant is counted on the feature level (
CachedFeature
), while each actual variant is counted on the individual variant (CachedVariant
).This PR introduces a new
CachedVariant
struct. That struct wraps all the data that exists inapi::Variant
and adds a newcount
property.Additionally, I've introduced a
variants_metrics
method for theCachedFeature
struct serializes its variant metrics into hash map.Discussion points
Manual counting of variants
I've left the task of counting variants as a manual job, keeping in line with how
yes
andno
are counted. We could add anincrement_count
method to theCachedVariant
struct, but I don't know if it's worth it.Benches
Running the benches locally, I see no significant changes between the main branch and this. All differences appear to be within the noise threshold.
The one benchmark that is the most interesting is probably the "batch/parallel unknown-features(str)", which took a pretty significant hit in #57.
Running it with the latest changes in this branch (against main), actually resulted in improved performance. This is probably noise, but it at least seems to indicate that this is fairly safe.