Skip to content

chore: Move VersionCollector to collectors directory - #1427

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

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

Conversation

@ArthurSens

Copy link
Copy Markdown
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

SuperQ commented Jan 9, 2024

Copy link
Copy Markdown
Member

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

SuperQ commented Jan 11, 2024

Copy link
Copy Markdown
Member

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
Copy Markdown
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
… code.

Signed-off-by: Arthur Silva Sens <arthur.sens@coralogix.com>
Signed-off-by: Arthur Silva Sens <arthur.sens@coralogix.com>
@ArthurSens
ArthurSens force-pushed the move-versioncollector branch from 7cce798 to 7cf8416 Compare January 14, 2024 19:05

@kakkoyun kakkoyun left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
Member

💪🏽

@SuperQ

SuperQ commented Jan 15, 2024

Copy link
Copy Markdown
Member

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