-
Notifications
You must be signed in to change notification settings - Fork 21
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 an option to expose Prometheus metrics via http/s in the forwarder-vpp #1137
Add an option to expose Prometheus metrics via http/s in the forwarder-vpp #1137
Conversation
c2a864c
to
cae240c
Compare
main.go
Outdated
collectorAddress := cfg.OpenTelemetryEndpoint | ||
spanExporter := opentelemetry.InitSpanExporter(ctx, collectorAddress) | ||
metricExporter := opentelemetry.InitPrometheusMetricExporter(ctx) | ||
o := opentelemetry.Init(ctx, spanExporter, metricExporter, cfg.Name) | ||
defer func() { | ||
if err = o.Close(); err != nil { | ||
log.FromContext(ctx).Error(err.Error()) | ||
} | ||
}() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious. Why do we need to use opentemeltry in the prometheus branch?
b95ca3a
to
e24ec63
Compare
Could you please rebase the branch to the latest sdk? |
55cc797
to
402950b
Compare
Hi @denis-tingaikin, I have rebased the branch to the latest sdk. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added one comment, in general looks good.
internal/config/config.go
Outdated
PrometheusIP string `default:"" desc:"Prometheus IP address" split_words:"true"` | ||
PrometheusPort uint16 `default:"8081" desc:"Prometheus port to use" split_words:"true"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be combined to PrometheusListenOn by analogy with ListenOn
, PprofListenOn
.
402950b
to
e387185
Compare
…r-vpp Signed-off-by: Botond Szirtes <botond.szirtes@est.tech>
e387185
to
cdc9953
Compare
Hi @denis-tingaikin, Thanks! I've made the modification as you recommended. |
…d-forwarder-vpp@main PR link: networkservicemesh/cmd-forwarder-vpp#1137 Commit: bed7f0f Author: Botond Szirtes Date: 2024-08-27 14:14:23 +0200 Message: - Add an option to expose Prometheus metrics via http/s in the forwarder-vpp (#1137) Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
…d-forwarder-vpp@main (#12270) PR link: networkservicemesh/cmd-forwarder-vpp#1137 Commit: bed7f0f Author: Botond Szirtes Date: 2024-08-27 14:14:23 +0200 Message: - Add an option to expose Prometheus metrics via http/s in the forwarder-vpp (#1137) Signed-off-by: NSMBot <nsmbot@networkservicmesh.io> Co-authored-by: NSMBot <nsmbot@networkservicmesh.io>
Description
Added a section in the main for initializing Prometheus and extended the config with the required variables.
Issue link
networkservicemesh/sdk#1652
How Has This Been Tested?
Types of changes