Rename metrics.proto to io_prometheus_client_metrics.proto#45
Rename metrics.proto to io_prometheus_client_metrics.proto#45
Conversation
This should solve naming collision problems, apparently caused by having a very generically named file directly in the root directory of the repository. Fixes #44 (which also provides more context). Signed-off-by: beorn7 <beorn@grafana.com>
|
I believe it will solve the issue. Thanks! |
bwplotka
left a comment
There was a problem hiding this comment.
Thanks, it's weird some systems depends on it, but no harm on our side. Never heard about the requirements for proto file name... Lgtm!
|
So I talked to people and looks like this might be technically a "breaking change" if someone imports this file directly (it copies that manually, use bazel etc), I think we might want to at least note it in the CHANGELOG.
Source: https://bufbuild.slack.com/archives/CRZ680FUH/p1623089122150800 |
|
If you're going to do this, it would be way nicer to do this as |
|
|
|
I understood the conversation linked in #44 such that the file name itself must be unique, too. But I'll do whatever the protobuf experts agree on. WRT CHANGELOG and breaking changes: There is no CHANGELOG. This repo isn't really versioned, and it is supposed to be used only by prometheus/client_golang and prometheus/pushgateway. I would be surprised if there is a relevant number of people importing the metrics.proto file. (I would actually be genuinely interested in their use case.) |
|
The filename doesn't need to be unique, the path to it does. So |
|
Oh, thanks @beorn7 I somehow thought we are in client_golang space 🙈
right, so we could just change the path we store it in? |
|
Everyone will be a lot happier if we just |
Envoy imports this file in a ton of places |
|
@bufdev I'm just following instructions of others here. Could you create a PR for this repo, which we can then feed to those that told me I have to pick a unique name? We can then see what arguments they present, or if it all turns out to be a misunderstanding.
Could you give me a tl;dr version why? Is this for Prometheus instrumentation without using an instrumentation library? |
|
I put up #46 to try to help with this. I don't work on the envoy team, but they have a lot of uses of this - just do a simple grep of |
|
Interesting… they mostly seem to embed Prometheus metrics in other proto messages for their gRPC stuff. And thanks for #46. |
This should solve naming collision problems, apparently caused by
having a very generically named file directly in the root directory of
the repository.
Fixes #44 (which also provides more context).
@newhook does this solve the problem you ran into?
@bwplotka @kakkoyun please review as this repo is only used by prometheus/client_golang. I believe in the way I changed things here, client_golang shouldn't notice any difference.