Remove config dependency in discovery

[fabxc]

Currently the discovery packages relies on config via config.TargetGroup. This is not the best design for reusability and the type should be moved into the package itself.

To be discussed whether we just have the type twice and map between it or import discovery into config.

[cstyan]

Are there not more dependencies here? ProvidersFromConfig makes use of config.ServiceDiscoverConfig

The import option seems to make more sense to me, if we have the TargetGroup type in two packages and map between the two we need to maintain the type in two separate places, plus the code to map between the two.

Is there anything I'm missing here?

[shubheksha]

I'll pick this up!

[krasi-georgiev]

@shubheksha I have done some work around discovery and config recently so ping me on IRC (prometheus-dev) if you need any pointers.

[shubheksha]

@fabxc, as part of this change, will the SD structs be moved to their respective packages as implied by the title as well? Or are we only concerning ourselves with config.TargetGroup?

[krasi-georgiev]

@shubheksha I don't think there is a definitive answer here.
I think some devs favour a global config package where others(me included) prefer package independence.

I think @brian-brazil can also share some more context here.

[juliusv]

@krasi-georgiev But at least the issue title implies getting rid of the SD->config dependency, which would naturally have to include moving any SD-related config structs to the SD packages as well.

[krasi-georgiev]

Yes I agree with this as well. Just have some faint memory that there were at least 2 opinions on this.

[brian-brazil]

I don't recall any disagreements here. Ultimately if we want SD to be usable as a separate component then the config bits need to move out of the config package.

[fabxc]

Yea, moving them to the respective packages is a good first step.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.