Skip to content

Conversation

@jacksontj
Copy link
Contributor

The fields of the Desc struct are unexported so other packages can't
reach in and change the value, but sometimes it is useful to read the
value of these fields (such as writing a registry/gatherer/etc.)

The fields of the Desc struct are unexported so other packages can't
reach in and change the value, but sometimes it is useful to *read* the
value of these fields (such as writing a registry/gatherer/etc.)
@jacksontj
Copy link
Contributor Author

cc: @beorn7

@beorn7
Copy link
Member

beorn7 commented Jul 20, 2017

I'm reworking this for #222 . I don't want to expose an internal structure in the main repo that is doomed to get changed in a couple of months.

I hope you understand. If you really need this in the meantime, feel free to work off a fork.

@beorn7 beorn7 closed this Jul 20, 2017
@beorn7
Copy link
Member

beorn7 commented Jul 20, 2017

I have read a bit of the context now.

Perhaps it is interesting to know that I'm also working on #47 to allow more lenient metrics exposition. However, the whole library is designed to not ever allow the exposition of inconsistent metrics. There are various ways of dealing with those errors, but if this PR is meant to facilitate writing registries that allow that, then I'm afraid that's completely against the basic principle of this library and Prometheus as a whole.

@jacksontj
Copy link
Contributor Author

jacksontj commented Jul 20, 2017

Adding getters() here facilitates creating any registry that isn't part of the prometheus client library package. Now if you want to implement the Gatherer interface you have to maintain a map of Desc -> Metric, since the Gather() method needs to return ([]*dto.MetricFamily, error)

As for the comment of

then I'm afraid that's completely against the basic principle of this library and Prometheus as a whole.

This is not necessarily true. The goal here is to facilitate migrations from other metrics systems (granted with other rules) to prometheus. The statsd_exporter (which I'm primarily doing this work for) is a wonderful mechanism to get people into prometheus-- as its a low-cost, low-effort mechanism to try out prometheus.

Thanks for the quick reply, hopefully we can get this (or something like it) in. If not I can work around this in the statsd_exporter (the PR is linked). IMO this PR is helpful if anyone wants to implement the Gatherer/Registerer interfaces-- which presumably is "supported" since they are interfaces.

cc: @beorn7

@brian-brazil
Copy link
Contributor

The goal here is to facilitate migrations from other metrics systems (granted with other rules) to prometheus.

You should not need to make any changes to a client library's Registry to do this, the existing Collector interface should cover you as it permits ingestion of arbitrary valid data. Usually when someone feels the need to rewrite the Registry, they've missed this aspect of the API design.

@beorn7
Copy link
Member

beorn7 commented Jul 21, 2017

There are many ways (and use cases) to implement Gatherer and Registerer without a need to meddle with Desc internals. If you want to go all low level, you could even create dto.MetricFamily protobuf messages on your own and return them.

However, my immediate concern here is that the Desc internals are not stable at the moment. Once they are, we can revisit your use case and find the best way to deal with it.

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.

3 participants