Skip to content

chore: Move VersionCollector to collectors directory#1427

Merged
kakkoyun merged 2 commits intomainfrom
move-versioncollector
Jan 15, 2024
Merged

chore: Move VersionCollector to collectors directory#1427
kakkoyun merged 2 commits intomainfrom
move-versioncollector

Conversation

@ArthurSens
Copy link
Member

Moving the newly added Version Collector to the collectors' directory. It's being kept in a separate package because, as @SuperQ mentioned, it uses init() to collect some information. We don't want to do that unless it is explicit to importers

I also added an example in a separate commit, happy to move it to a separate PR if that's preferred.

@ArthurSens ArthurSens force-pushed the move-versioncollector branch from d7cbb71 to bd1d5af Compare January 9, 2024 00:14
@SuperQ
Copy link
Member

SuperQ commented Jan 9, 2024

I don't think this package belongs in the collectors directory because the primary use case of this package is to provide a structured version information. The version metric is an extra feature, not the primary feature.

Honestly, I think we were wrong in moving this to client_golang. The version information package probably makes more sense in exporter-toolkit.

@SuperQ SuperQ requested a review from roidelapluie January 9, 2024 10:11
@SuperQ
Copy link
Member

SuperQ commented Jan 11, 2024

As a counter-proposal, we could keep version in common. But move the NewCollector() function here.

In order to do this, I've opened prometheus/common#563 to make the helper function public.

@ArthurSens ArthurSens force-pushed the move-versioncollector branch from bd1d5af to 8a3bec2 Compare January 11, 2024 23:24
@ArthurSens
Copy link
Member Author

I've changed the PR to continue using prometheus/common, but still depends on prometheus/common#565

@ArthurSens ArthurSens force-pushed the move-versioncollector branch 3 times, most recently from c1fb44d to 7cce798 Compare January 12, 2024 21:39
Arthur Silva Sens added 2 commits January 14, 2024 16:05
@ArthurSens ArthurSens force-pushed the move-versioncollector branch from 7cce798 to 7cf8416 Compare January 14, 2024 19:05
Copy link
Member

@kakkoyun kakkoyun left a comment

Choose a reason for hiding this comment

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

@kakkoyun kakkoyun merged commit 38631c6 into main Jan 15, 2024
@kakkoyun kakkoyun deleted the move-versioncollector branch January 15, 2024 09:07
@bwplotka
Copy link
Member

💪🏽

@SuperQ
Copy link
Member

SuperQ commented Jan 15, 2024

Nice

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.

4 participants