Skip to content

Add Feast Serving histogram metrics - #1240

Merged
feast-ci-bot merged 4 commits into
feast-dev:masterfrom
terryyylim:add-online-retrieval-metrics
Dec 23, 2020
Merged

Add Feast Serving histogram metrics#1240
feast-ci-bot merged 4 commits into
feast-dev:masterfrom
terryyylim:add-online-retrieval-metrics

Conversation

@terryyylim

Copy link
Copy Markdown
Member

Signed-off-by: Terence terencelimxp@gmail.com

What this PR does / why we need it:
This PR adds histogram metrics for Feast serving, which would allow us to determine:

  • amount of entity rows per request (buckets)
  • amount of features per request (buckets)
  • amount of feature tables per request (buckets)

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

Users can gather entity row, features and feature table requested metrics in Feast Serving.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit: looks like you use featureTableRefs just for counting distinct strings, consider doing

long countDistinct = featureReferences.stream().map(...)
                        .distinct()
                        .count();
....
        .observe(Double.valueOf(countDistinct));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

oh, there could be much more :)

Let's do 1, 2, 5, 10, 20, 50, 100, 200

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

1, 2, 5, 10, 15, 20, 30, 50

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

1, 2, 5, 10, 20

@feast-ci-bot

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pyalex, terryyylim

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@pyalex

pyalex commented Dec 23, 2020

Copy link
Copy Markdown
Collaborator

/lgtm

Signed-off-by: Terence <terencelimxp@gmail.com>
Signed-off-by: Terence <terencelimxp@gmail.com>
Signed-off-by: Terence <terencelimxp@gmail.com>
@terryyylim
terryyylim force-pushed the add-online-retrieval-metrics branch from 7182d88 to b1f0e5c Compare December 23, 2020 05:28
@feast-ci-bot feast-ci-bot removed the lgtm label Dec 23, 2020
Signed-off-by: Terence <terencelimxp@gmail.com>
@terryyylim
terryyylim force-pushed the add-online-retrieval-metrics branch from f4cb648 to 130e58d Compare December 23, 2020 05:33
@terryyylim

Copy link
Copy Markdown
Member Author

/retest

@pyalex

pyalex commented Dec 23, 2020

Copy link
Copy Markdown
Collaborator

/lgtm

@feast-ci-bot
feast-ci-bot merged commit 6db1ec0 into feast-dev:master Dec 23, 2020
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