Skip to content

feat: Bucket feature count labels in latency metrics - #6811

Open
jingqian-liu wants to merge 3 commits into
feast-dev:masterfrom
jingqian-liu:fix/feature-count-metric-binning
Open

jingqian-liu wants to merge 3 commits into
feast-dev:masterfrom
jingqian-liu:fix/feature-count-metric-binning

Conversation

@jingqian-liu

Copy link
Copy Markdown

What this PR does / why we need it:

The feature_count label on
feast_feature_server_request_latency_seconds currently uses the exact
number of requested features. For workloads with widely varying feature
counts, this can create a large number of Prometheus time series and
increase metric cardinality.

This PR:

  • Buckets feature_count label values using configurable boundaries.
  • Adds metrics.feature_count_bins, defaulting to [10, 50, 200].
  • Produces the default labels 0, 1-10, 11-50, 51-200, and 201+.
  • Preserves 0 as a separate label for zero-feature requests.
  • Keeps the exact feature count in audit logs; only the Prometheus latency
    metric label is bucketed.
  • Validates that configured bucket boundaries are positive and strictly
    increasing.
  • Documents the new configuration option.

For example:

feature_server:
  type: local
  metrics:
    enabled: true
    feature_count_bins: [5, 20]

@jingqian-liu
jingqian-liu requested a review from a team as a code owner September 2, 2026 20:27
@codecov-commenter

codecov-commenter commented Sep 3, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 47.44%. Comparing base (640ac70) to head (7107102).
⚠️ Report is 1 commits behind head on master.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6811      +/-   ##
==========================================
+ Coverage   47.41%   47.44%   +0.02%     
==========================================
  Files         421      421              
  Lines       52249    52273      +24     
  Branches     7582     7587       +5     
==========================================
+ Hits        24776    24801      +25     
  Misses      25708    25708              
+ Partials     1765     1764       -1     
Flag Coverage Δ
go-feature-server 30.58% <ø> (ø)
python-unit 48.77% <100.00%> (+0.02%) ⬆️
Files with missing lines Coverage Δ
sdk/python/feast/feature_server.py 63.63% <100.00%> (+0.71%) ⬆️
.../python/feast/infra/feature_servers/base_config.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 640ac70...7107102. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jingqian-liu

Copy link
Copy Markdown
Author

The macOS Python 3.12 unit-test job is still failing after the rerun. I checked the failure, and the only failing test is test_module_level_fips_sets_env_before_pyarrow_import, which times out after 60 seconds while importing feast.offline_server. All feature-server-related tests and the other Python/macOS/Linux unit-test jobs pass, so this appears unrelated to the changes in this PR.

@ntkathole
ntkathole force-pushed the fix/feature-count-metric-binning branch from bac23ab to 9b57eb5 Compare September 9, 2026 10:54
Comment thread sdk/python/feast/feature_server.py Outdated
@jingqian-liu
jingqian-liu force-pushed the fix/feature-count-metric-binning branch from 8b13f85 to 3185b9b Compare September 14, 2026 22:12
@jingqian-liu

Copy link
Copy Markdown
Author

Rebased onto current master to pick up #6835, which fixes the CPU wheel hash mismatch and stabilizes the FIPS import test seen in the previous CI runs. CI is rerunning now. Thanks!

@ntkathole
ntkathole force-pushed the fix/feature-count-metric-binning branch from 3185b9b to 6ec94bc Compare September 16, 2026 06:56
@ntkathole

Copy link
Copy Markdown
Member

@jingqian-liu can you please resolve conflicts ?

Signed-off-by: Jingqian Liu <ljqstella@gmail.com>
Signed-off-by: Jingqian Liu <ljqstella@gmail.com>
Signed-off-by: Jingqian Liu <ljqstella@gmail.com>
@jingqian-liu
jingqian-liu force-pushed the fix/feature-count-metric-binning branch from 6ec94bc to 7107102 Compare September 17, 2026 17:57
@jingqian-liu

Copy link
Copy Markdown
Author

@jingqian-liu can you please resolve conflicts ?

Resolved the conflicts with current master and pushed the updated branch. Thanks!

@jingqian-liu

Copy link
Copy Markdown
Author

@jingqian-liu can you please resolve conflicts ?

Hi @ntkathole, the conflicts are resolved and the branch has been rebased onto current master. Since master is moving quickly, if everything looks good after the workflows are approved, would you mind merging this when convenient? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants