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 simple stats API #242

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

junr03
Copy link

@junr03 junr03 commented Jul 11, 2024

This PR exposes the concept of Metrics with a high level API. It takes liberal inspiration from the Simple<Counter,Gauge,Histogram> in the CPP SDK.

Users of the Rust SDK will now be able to create and manipulate simple Counters, Gauges, and Histograms that exist in the Envoy Runtime.

Note that this change is more generic than #229 given that it exposes the concept of Counters, Gauges, and Histograms as a first class implementation, rather than interacting directly with the metrics host calls via the Root Context.

cc @casimiro

Signed-off-by: José Ulises Niño Rivera <[email protected]>
@junr03 junr03 requested a review from PiotrSikora as a code owner July 11, 2024 18:32
Copy link

@adilhafeez adilhafeez left a comment

Choose a reason for hiding this comment

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

It would be great if you can update one of the examples and show use of stats there.

src/stats.rs Outdated Show resolved Hide resolved
Signed-off-by: José Ulises Niño Rivera <[email protected]>
Copy link
Contributor

@Swaagie Swaagie left a comment

Choose a reason for hiding this comment

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

I've actually tested this local on a project and it works nicely. @PiotrSikora what is your opinion on this vs the other metric PR?

@@ -0,0 +1,32 @@
## Proxy-Wasm plugin example: HTTP headers
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
## Proxy-Wasm plugin example: HTTP headers
## Proxy-Wasm plugin example: Metrics

@@ -0,0 +1,32 @@
## Proxy-Wasm plugin example: HTTP headers

Proxy-Wasm plugin that logs HTTP request/response headers.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please update this as well.

### Building

```sh
$ cargo build --target wasm32-wasi --release
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$ cargo build --target wasm32-wasi --release
$ cargo build --target wasm32-wasip1 --release


services:
envoy:
image: envoyproxy/envoy:v1.24-latest
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
image: envoyproxy/envoy:v1.24-latest
image: envoyproxy/envoy:v1.31-latest

direct_response:
status: 200
body:
inline_string: "Request /hello and be welcomed!\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

Leave a more specific helpful message that explains what header to set. In addition IMHO it might be useful to have the WASM filter return the current values of metrics in the response without having to check the /stats endpoint. This would make the example more Envoy agnostic as well.

use crate::types;

#[derive(Copy, Clone)]
pub struct Counter {
Copy link
Contributor

Choose a reason for hiding this comment

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

Like this abstraction 🚀

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.

3 participants