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

libpromhttp: init at 0.1.1 #91323

Closed
wants to merge 1 commit into from
Closed

libpromhttp: init at 0.1.1 #91323

wants to merge 1 commit into from

Conversation

@cfsmp3
Copy link
Contributor

cfsmp3 commented Jun 23, 2020

Motivation for this change

Build the HTTP endpoint for prometheus C library. It's needed to expose the metrics.

Things done

Simple derivation that is based on its sister "libprom", pretty much identical to it except for the dependency to libprom itself.

It's an addition, not a change, so it does not break anything.

  • Built on platform(s)

    • NixOS
    • macOS
    • other Linux distributions
  • Determined the impact on package closure size (by running nix path-info -S before and after)

  • Ensured that relevant documentation is up to date

  • Fits CONTRIBUTING.md.

@cole-h
cole-h approved these changes Jun 23, 2020
Copy link
Member

cole-h left a comment

Diff LGTM, builds fine.

[5 built, 2 copied (0.3 MiB), 0.1 MiB DL]
https://github.com/NixOS/nixpkgs/pull/91323
1 package built:
libpromhttp
@jtojnar
Copy link
Contributor

jtojnar commented Jun 23, 2020

Hmm, looks like it will use the headers from the prom directory:

https://github.com/digitalocean/prometheus-client-c/blob/0c15e7e45ad0c3726593591fdd7d8f2fde845fe3/promhttp/CMakeLists.txt#L49

Maybe we should delete it so it has to choose ones from the package – of course the proper solution is linking against a target obtained by find_package but not sure if you are up for that.

@cfsmp3
Copy link
Contributor Author

cfsmp3 commented Jun 23, 2020

Maybe we should delete it so it has to choose ones from the package – of course the proper solution is linking against a target obtained by find_package but not sure if you are up for that.

If you mean by sending digitalocean a PR to improve their CMakelist, probably not at least immediately. It's just too big a detour from my current to-do.

@cfsmp3
Copy link
Contributor Author

cfsmp3 commented Jun 23, 2020

@jtojnar could this be merged?

@purcell
Copy link
Contributor

purcell commented Jul 1, 2020

Collaborating with @cfsmp3, this has been reformulated as #91908, which should then probably be preferred to this PR.

@cole-h
Copy link
Member

cole-h commented Jul 2, 2020

In lieu of the above comment, I'll close this PR and review the other one :-)

@cole-h cole-h closed this Jul 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.