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

Collector pattern #74

Closed
wants to merge 4 commits into from
Closed

Conversation

vladvasiliu
Copy link

This is an implementation of a Collector trait for collecting all the Metric Families from one or more Registries to encode them all at once.

For example, this allows exporting metrics by combining a "long-running" Registry with a per-scrape Registry.

This is in response to #29, #49 and #73.

Implementation is rather crude, but the out-facing interface is basically unchaged.

Signed-off-by: Vlad Vasiliu <vladvasiliun@yahoo.fr>
Signed-off-by: Vlad Vasiliu <vladvasiliun@yahoo.fr>
Signed-off-by: Vlad Vasiliu <vladvasiliun@yahoo.fr>
@mxinden
Copy link
Member

mxinden commented Aug 10, 2022

I am sorry for the late reply. This is a neat solution. I did not think about abstracting a Registry.

Unfortunately a user can not use the Collector abstraction in combination with the Registry::sub_registry_with_xxx mechanism. I would expect this to be useful for users. Would you agree? This is similar to the suggestion in #29 (comment).

I have to give this more thought. Have you had the chance to play with this approach instead?

@vladvasiliu
Copy link
Author

Unfortunately a user can not use the Collector abstraction in combination with the Registry::sub_registry_with_xxx mechanism. I would expect this to be useful for users. Would you agree?

I'd have to get back to looking at this. Right now, it's not clear to me why that wouldn't work. The collector delegates the actual encoding to the Registry, so it's the Registry's business to deal with its subregistries, and I haven't touched that. There's a test checking for this, which passes.

fn encode_counter_family_with_prefix_with_label() {

I'd have to check to make sure it doesn't somehow sidestep the issue.

This is similar to the suggestion in #29 (comment).

That was actually my inspiration for this. The overall idea was to allow encode to take a bunch of Collectors instead of a Registry. But the user shouldn't have to deal with a Collector directly, just use the Registry as usual.

@mxinden
Copy link
Member

mxinden commented Aug 29, 2022

Cross referencing alternative proposal: #82

@mxinden
Copy link
Member

mxinden commented Dec 29, 2022

Implemented through #82. Let me know what you think. Closing here.

@mxinden mxinden closed this Dec 29, 2022
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