feat(go): Implement metrics and tracing for http and grpc servers by luisazofracabify · Pull Request #5925 · feast-dev/feast

luisazofracabify · 2026-01-30T11:53:46Z

Description

This PR significantly improves the observability and reliability of the Feast Go Feature Server by implementing comprehensive Prometheus metrics, and robust configuration options for both HTTP and gRPC modes. It addresses issues with hardcoded ports, inconsistent metric exposure, and potential race conditions during shutdown.

Changes

1. Prometheus Metrics Instrumentation

Unified Metrics Server: Implemented a dedicated, configurable HTTP server (default port :9090) for serving metrics in both HTTP and gRPC modes. This unifies the observability strategy.
HTTP Server Instrumentation: Added metricsMiddleware in internal/feast/server/http_server.go to track:
- Request duration (http_request_duration_seconds)
- Request counts by method, path, and status (http_requests_total)
- Instrumented the /health endpoint for readiness probe visibility.
gRPC Metrics: Integrated go-grpc-prometheus to fully expose standard gRPC server metrics.

2. Tracing Enhancements

Dynamic Service Name: Improved the existing OpenTelemetry integration by adding support for the OTEL_SERVICE_NAME environment variable (defaults to FeastGoFeatureServer). This allows proper service identification in distributed tracing systems without code changes.

3. Server Configuration & Reliability

Configurable Metrics Port: Added -metrics-port flag to main.go.
Graceful Shutdown: Implemented sync.WaitGroup in StartHttpServer to ensure clean shutdown of the metrics server and logging services, matching the robust behavior of StartGrpcServer.
Code Cleanup: Refactored ServerStarter interfaces and removed unused legacy code.

How Has This Been Tested?

Verification

Confirmed /metrics endpoint accessibility on custom ports.
Verified /health and request metrics counters.
Validated status 200 recording fix.
Tested OTEL_SERVICE_NAME env var reflects in traces.
Verified clean shutdown logs (SIGINT/SIGTERM).

Checklist

My code follows the code style of this project.
I have added tests that prove my fix is effective or that my feature works.
I have updated the dependencies (go.mod / go.sum).
All new and existing tests passed.

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

devin-ai-integration

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 6 additional flags.

devin-ai-integration

Devin Review found 1 new potential issue.

View issue and 9 additional flags in Devin Review.

go/main.go

devin-ai-integration

Devin Review found 1 new potential issue.

View issue and 12 additional flags in Devin Review.

go/main.go

PepeluDev · 2026-02-05T14:27:26Z

go/feast-server

@luisazofracabify Please remove this binary file from the commit or add it to the .gitignore file if you find it valuable.

PepeluDev · 2026-02-05T14:32:37Z

go/internal/feast/metrics/metrics.go

+)
+
+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"`


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

PepeluDev · 2026-02-05T14:38:27Z

go/internal/feast/metrics/metrics_test.go

I would remove this file. Such a test does not actually test that metrics actually record values.

PepeluDev · 2026-02-05T14:42:41Z

go/internal/feast/server/server_commons.go

Is this change in import libraries order needed? If not, please try to modify only files which are strictly necessary for your contribution.

go/main.go

devin-ai-integration

Devin Review found 1 new potential issue.

View 9 additional findings in Devin Review.

go/internal/feast/server/http_server.go

PepeluDev · 2026-02-09T07:53:26Z

go/main.go

 	flag.IntVar(&port, "port", port, "Specify a port for the server")
 	flag.Parse()

+	metrics.InitMetrics()


do you need this? AFAIK the init() function in Go is automatically called by the Go runtime when the package is imported. It doesn't need to be explicitly called anywhere.

Thanks for the suggestion, I applied the fix as metrics.InitMetrics() was not really needed

devin-ai-integration

Devin Review found 1 new potential issue.

View 10 additional findings in Devin Review.

go/main.go

PepeluDev · 2026-02-11T07:30:11Z

go/main.go

 		if loggingService != nil {
 			loggingService.Stop()
 		}
+


Beware of trailing spaces in this line and 208.

Signed-off-by: Luis Azofra Begara <luis.azofra@cabify.com>

devin-ai-integration

Devin Review found 1 new potential issue.

View 16 additional findings in Devin Review.

go/main.go

Signed-off-by: Luis Azofra Begara <luis.azofra@cabify.com>

$@luisazofracabify$ luisazofracabify requested a review from a team as a code owner January 30, 2026 11:53

devin-ai-integration bot reviewed Jan 30, 2026

View reviewed changes

$@luisazofracabify$ luisazofracabify force-pushed the feat/go-server-observability branch from 82aca5b to b876dba Compare January 30, 2026 11:58

devin-ai-integration bot reviewed Jan 30, 2026

View reviewed changes

go/main.go Show resolved Hide resolved

$@luisazofracabify$ luisazofracabify force-pushed the feat/go-server-observability branch from b876dba to 8b5cd4b Compare January 30, 2026 12:30

devin-ai-integration bot reviewed Jan 30, 2026

View reviewed changes

go/main.go Outdated Show resolved Hide resolved

$@luisazofracabify$ luisazofracabify force-pushed the feat/go-server-observability branch from 8b5cd4b to 847393e Compare January 30, 2026 12:39

$@luisazofracabify$ luisazofracabify changed the title ~~feat(go): implement metrics and tracing for http and grpc servers~~ feat(go): Implement metrics and tracing for http and grpc servers Feb 2, 2026

PepeluDev reviewed Feb 5, 2026

View reviewed changes

go/main.go Show resolved Hide resolved

$@luisazofracabify$ luisazofracabify force-pushed the feat/go-server-observability branch 2 times, most recently from 655cd02 to 34de413 Compare February 6, 2026 09:01

devin-ai-integration bot reviewed Feb 9, 2026

View reviewed changes

go/internal/feast/server/http_server.go Show resolved Hide resolved

PepeluDev reviewed Feb 9, 2026

View reviewed changes

$@luisazofracabify$ luisazofracabify force-pushed the feat/go-server-observability branch from e7a22fc to 2e4c1b3 Compare February 9, 2026 08:21

devin-ai-integration bot reviewed Feb 9, 2026

View reviewed changes

go/main.go Outdated Show resolved Hide resolved

$@luisazofracabify$ luisazofracabify force-pushed the feat/go-server-observability branch from a5345c2 to f5449af Compare February 9, 2026 08:37

PepeluDev reviewed Feb 11, 2026

View reviewed changes

go/main.go Outdated

if loggingService != nil {

loggingService.Stop()

}

Copy link

Contributor

PepeluDev Feb 11, 2026

Choose a reason for hiding this comment

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

Beware of trailing spaces in this line and 208.

$@luisazofracabify$ luisazofracabify force-pushed the feat/go-server-observability branch 2 times, most recently from db00427 to b9f99a6 Compare February 11, 2026 08:30

$@luisazofracabify$

feat(go): implement metrics and tracing for http and grpc servers

22a253b

Signed-off-by: Luis Azofra Begara <luis.azofra@cabify.com>

$@luisazofracabify$ luisazofracabify force-pushed the feat/go-server-observability branch from b9f99a6 to 7b552a9 Compare February 11, 2026 12:16

luisazofracabify added 2 commits February 11, 2026 13:20

$@luisazofracabify$

fix(server): improve metrics, config, and shutdown logic

1664ae5

Signed-off-by: Luis Azofra Begara <luis.azofra@cabify.com>

$@luisazofracabify$

chore: update go.sum after rebase

1bf7277

Signed-off-by: Luis Azofra Begara <luis.azofra@cabify.com>

$@luisazofracabify$ luisazofracabify force-pushed the feat/go-server-observability branch from 7b552a9 to 1bf7277 Compare February 11, 2026 12:21

$@luisazofracabify$

docs: improve README instructions for metrics and tracing

816423b

Signed-off-by: Luis Azofra Begara <luis.azofra@cabify.com>

$@luisazofracabify$ luisazofracabify force-pushed the feat/go-server-observability branch from 5d6cc58 to 816423b Compare February 11, 2026 13:27

devin-ai-integration bot reviewed Feb 11, 2026

View reviewed changes

go/main.go Show resolved Hide resolved

$@luisazofracabify$

fix(server): resolve potential deadlock during shutdown

41d58e2

Signed-off-by: Luis Azofra Begara <luis.azofra@cabify.com>

Conversation

luisazofracabify commented Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

1. Prometheus Metrics Instrumentation

2. Tracing Enhancements

3. Server Configuration & Reliability

How Has This Been Tested?

Verification

Checklist

Why gotoprom

Uh oh!

devin-ai-integration bot left a comment

Choose a reason for hiding this comment

✅ Devin Review: No Issues Found

Uh oh!

devin-ai-integration bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

devin-ai-integration bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

PepeluDev Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PepeluDev Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

PepeluDev Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

PepeluDev Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

devin-ai-integration bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

PepeluDev Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

luisazofracabify Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

PepeluDev Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

$@luisazofracabify$ luisazofracabify commented Jan 30, 2026 •

edited

Loading

PepeluDev Feb 5, 2026 •

edited

Loading

$@luisazofracabify$ luisazofracabify Feb 9, 2026