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 support for push-gateway and OTLP #67

Merged
merged 1 commit into from
Sep 15, 2023
Merged

Add support for push-gateway and OTLP #67

merged 1 commit into from
Sep 15, 2023

Conversation

gagbo
Copy link
Member

@gagbo gagbo commented Sep 4, 2023

Description

Add support for using "push"-based workflows with metrics:

  • When using Prometheus for metric collection (...autometrics-go/prometheus), autometrics will push the metrics to a given Push Gateway
  • When using OpenTelemetry for metric collection (...autometrics-go/otel), autometrics will push OTLP metrics periodically to a collector.

This also forcefully updates opentelemetry libraries to 1.17/0.40

Checklist

  • The CHANGELOG is updated.
  • The open-telemetry example in repository works fine:
    • the docker compose file is valid
    • the application compiles and run
    • alerts are getting triggered in Prometheus
  • The prometheus example in repository works fine:
    • the docker compose file is valid
    • the application compiles and run
    • alerts are getting triggered in Prometheus
    • exemplars are accessible on the graph

@gagbo gagbo force-pushed the otlp_push branch 2 times, most recently from 62b66d3 to d7d683c Compare September 5, 2023 06:59
@gagbo gagbo changed the title WIP: Add support for push-gateway and OTLP Add support for push-gateway and OTLP Sep 5, 2023
@gagbo gagbo force-pushed the otlp_push branch 3 times, most recently from 6822648 to 0f137e5 Compare September 5, 2023 07:58
@gagbo gagbo marked this pull request as ready for review September 5, 2023 07:59
@gagbo gagbo force-pushed the otlp_push branch 7 times, most recently from 8087511 to 5c28d1a Compare September 8, 2023 12:56
@gagbo gagbo force-pushed the otlp_push branch 2 times, most recently from 790f493 to 97bb5fa Compare September 15, 2023 09:32
// This is a useful function to use for setting a `job` key when
// pushing metrics
func GetOutboundIP() (net.IP, error) {
conn, err := net.Dial("tcp", "1.1.1.1:80")
Copy link
Member

Choose a reason for hiding this comment

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

eehh this may be nasty behavior imo, it is quite unexpected for the autometrics library to make connections to the wide internet. definitely need to document this behavior more obviously

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair enough

if pushConfiguration.CollectorURL == "" {
return nil, errors.New("invalid PushConfiguration: the CollectorURL must be set.")
}
autometrics.SetPushJobURL(pushConfiguration.CollectorURL)
Copy link
Member

Choose a reason for hiding this comment

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

what is the behavior if a url without /v1/metrics at the end gets inputted?

Copy link
Member Author

Choose a reason for hiding this comment

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

Golang prometheus library adds that part itself, so it should be fine not adding the extra specification here. In the docs they don't mention the v1 part, but the gateway URL is still supposed to just be the root of the gateway.

Forced to update opentelemetry-go as well as there was no way to bring
the update without also going into otel breaking changes.
@gagbo gagbo merged commit abf39f2 into main Sep 15, 2023
@gagbo gagbo deleted the otlp_push branch September 18, 2023 06:26
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.

2 participants