krakend / krakend-otel

KrakenD component for OpenTelemetry
Apache License 2.0
8 stars 10 forks source link

Prometheus Namespace #17

Open Mojtaba-saf opened 4 months ago

Mojtaba-saf commented 4 months ago

There should be a config option to allow usage of namespace for prometheus client. This will also help with compatibility and easier migration from krakend/krakend-opencensus.

It's also mention in Todecide of TODO list

in exporter/prometheus/prometheus.go we could add a config option that will add a prefix to all reported metrics (using the WithNamespace option).

I can do it but just had a question about it. should the WithNamespace option be a string variable and pass it to options or maybe a boolean that allows usage of another option named Namespace?

because it was called namespace in krakend/krakend-opencensus configs.

dhontecillas commented 4 months ago

@Mojtaba-saf : we are providing a dashboard for prometheus metrics, and adding that namespace / prefix will break the queries and will display nothing. But, having an optional namespace will make no harm.

I would add a Namespace field after this line: https://github.com/krakend/krakend-otel/blob/main/config/config.go#L125 , with a json field of namespace , and then put here https://github.com/krakend/krakend-otel/blob/main/exporter/prometheus/prometheus.go#L57 the WithNamespace option.

I let you prepare the PR and test the feature :)

Thanks for your interest in contributing to the repo.

Mojtaba-saf commented 4 months ago

thanks for the response. I will try my best to prepare it but it might take some time since I'm not a Golang developer I need to learn about testing in golang and write the tests for this feature.