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

Add support for filtering metrics in data streams #179

Merged

Conversation

tiyash-basu-frequenz
Copy link
Contributor

This change adds support for filtering metrics in data streams. The request messages for receiving data streams have now been extended to consist of a list of metrics to be streamed. This allows the user to request only the metrics they are interested in, instead of receiving all of them. If this list is empty, then no data will be streamed, and the service will return an error.

@tiyash-basu-frequenz tiyash-basu-frequenz added this to the v0.16.0 milestone Nov 16, 2023
@tiyash-basu-frequenz tiyash-basu-frequenz self-assigned this Nov 16, 2023
@tiyash-basu-frequenz tiyash-basu-frequenz requested a review from a team as a code owner November 16, 2023 13:20
@github-actions github-actions bot added part:docs Affects the documentation part:protobuf Affects the protocol buffer definition files labels Nov 16, 2023
Otherwise it makes it difficult to add links that make the lines go beyond
the 80 character limit.

Signed-off-by: Tiyash Basu <tiyash.basu@frequenz.com>
@tiyash-basu-frequenz
Copy link
Contributor Author

As discussed, the filters are optional now, so that users have a convenient way of fetching all metrics from a component, given that the list of available metrics can vary between components.

tiyash-basu-frequenz referenced this pull request Nov 17, 2023
This commit makes the filter for the data stream RPCs optional. This
is implemented by moving the ID and metrics fields into a nested
message, which is then added as the only field in the request.

This allows the filter to be omitted, which will result in the server
returning all metrics for all components or sensors.

This is helpful for clients to easily know which metrics are available
for a component or sensor. Otherwise, it may become difficult, given
that the list if available metrics can very from one component to
another.

Signed-off-by: Tiyash Basu <tiyash.basu@frequenz.com>
@tiyash-basu-frequenz
Copy link
Contributor Author

I updated the PR based on comments here.

I also changed the filter by moving the component_id out of it. The request returning all data for all components by default feels like overkill. Lemme know if this makes sense, and if I should move component_id back into the filter message.

Copy link
Contributor

@TalweSingh TalweSingh left a comment

Choose a reason for hiding this comment

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

2 optional comments. Otherwise LGTM!

proto/frequenz/api/microgrid/v1/microgrid.proto Outdated Show resolved Hide resolved
proto/frequenz/api/microgrid/v1/microgrid.proto Outdated Show resolved Hide resolved
TalweSingh
TalweSingh previously approved these changes Nov 20, 2023
This commit adds support for filtering metrics in data streams. The
request messages for receiving data streams have now been extended to
consist of a list of metrics to be streamed. This allows the user to
request only the metrics they are interested in, instead of receiving
all of them. If this list is empty, then no data will be streamed, and
the service will return an error.

The filter for the data stream RPCs are optional. This  is implemented
by putting the metrics field into a nested message, which is then
added as a field in the request message. This makes the filter
optional, so users can apply filters only when they intend to. This
allows the filter to be omitted, which will result in the server
returning all metrics for the given component or sensor.

Signed-off-by: Tiyash Basu <tiyash.basu@frequenz.com>
@tiyash-basu-frequenz tiyash-basu-frequenz added this pull request to the merge queue Nov 21, 2023
Merged via the queue into frequenz-floss:v0.x.x with commit b1ba87a Nov 21, 2023
11 checks passed
@tiyash-basu-frequenz tiyash-basu-frequenz deleted the 178_filter branch November 21, 2023 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
part:docs Affects the documentation part:protobuf Affects the protocol buffer definition files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Filter message and add it to the ReceiveComponentDataStreamRequest
4 participants