feat(go): Implement metrics and tracing for http and grpc servers#5925
feat(go): Implement metrics and tracing for http and grpc servers#5925luisazofracabify wants to merge 1 commit intofeast-dev:masterfrom
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
luisazofracabify
force-pushed
the
feat/go-server-observability
branch
from
82aca5b to
b876dba
Compare
luisazofracabify
force-pushed
the
feat/go-server-observability
branch
from
b876dba to
8b5cd4b
Compare
luisazofracabify
force-pushed
the
feat/go-server-observability
branch
from
8b5cd4b to
847393e
Compare
luisazofracabify
changed the title
go/feast-server
Outdated
There was a problem hiding this comment.
@luisazofracabify Please remove this binary file from the commit or add it to the .gitignore file if you find it valuable.
go/internal/feast/metrics/metrics.go
Outdated
| ) | ||
|
|
||
| var Metrics struct { | ||
| HttpDuration func(HttpLabels) TimeHistogram `name:"feast_http_request_duration_seconds" help:"Time taken to serve HTTP requests" buckets:".005,.01,.025,.05,.1,.25,.5,1,2.5,5,10"` |
There was a problem hiding this comment.
Please remove the feast_ preffix from each metric. Take into account it's already in the initialization with gotoprom.MustInit(&Metrics, "feast"). If you keep it you will end up with metrics with names like:
feast_feast_http_request_duration_seconds.
Anyway what i would do is to remove the feast_ at all. You could split http and grpc metrics in 2 separated Metrics structs. This way you woulkd have just:
http_request_duration_seconds and grpc_request_duration_seconds
There was a problem hiding this comment.
I would remove this file. Such a test does not actually test that metrics actually record values.
There was a problem hiding this comment.
Is this change in import libraries order needed? If not, please try to modify only files which are strictly necessary for your contribution.
luisazofracabify
force-pushed
the
feat/go-server-observability
branch
from
9abbdc9 to
6893255
Compare
Signed-off-by: Luis Azofra Begara <luis.azofra@cabify.com>
luisazofracabify
force-pushed
the
feat/go-server-observability
branch
from
655cd02 to
34de413
Compare
2 participants
Description
This PR addresses the lack of observability in the Feast Go Feature Server by implementing comprehensive metrics for both HTTP and gRPC endpoints, and integrating them with the existing OpenTelemetry tracing.
Changes
gotoprom(a type-safe wrapper for the official Prometheus client) to ensure standardized bucket configuration and label safety.internal/feast/metrics/metrics.goto define histograms and counters./metricsendpoint.internal/feast/server/http_server.goto capture request duration and status codes.http_server.go(instead ofmain.go) to maintain high cohesion within the server struct and ensure metrics are applied correctly to business endpoints while excluding health checks.main.goto capture gRPC specific metrics (latency, status codes, methods).9090to expose Prometheus metrics when running in gRPC mode.How Has This Been Tested?
/metricsendpoint returns valid Prometheus data./get-online-featuresincrements the counters and updates histograms.metrics_test.go.Checklist
go.mod/go.sum).Why gotoprom
It reduces boilerplate code for histograms and ensures type-safety for labels, preventing runtime panics due to mismatched label cardinality. It wraps the official prometheus client, so it's fully compatible