Targets not required in promtail config

[adityacs]

What this PR does / why we need it:
Targets are not required in promtail config. If defined in config we will just use that otherwise we will just add dummy host.

Which issue(s) this PR fixes:
Fixes #1929

Checklist

• Documentation added
• Tests updated

[adityacs]

@slim-bean Does this change has any impact on docker driver and any other module? I am not aware of other uses of this. Hope this change doesn't break anything

[owen-d]

Suggested change

	if !(len(tg.Targets) > 0) {
	if len(tg.Targets) == 0 {

[adityacs]

Perfect. Thanks @owen-d

[codecov-io]

Codecov Report

Merging #2026 into master will increase coverage by 0.00%.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##           master    #2026   +/-   ##
=======================================
  Coverage   63.82%   63.82%           
=======================================
  Files         133      133           
  Lines       10208    10211    +3     
=======================================
+ Hits         6515     6517    +2     
- Misses       3204     3206    +2     
+ Partials      489      488    -1     

Impacted Files	Coverage Δ
pkg/promtail/targets/filetargetmanager.go	0.00% <0.00%> (ø)
pkg/logql/evaluator.go	91.27% <0.00%> (+0.58%)	⬆️

[owen-d]

Thanks for the PR. Everything looks good, but I'm not informed enough on this part of the project to know if we want this change.

I have a few questions:

• Why do we assign dummy? These must be localhost in all cases as promtail can only scrape a file system.
• Do we want this shortcut? Is it worth the confusion of having identical-behaving configs, one which uses targets and one which omits it?

Part of me thinks we could make the case for removing the targets field entirely b/c it's not relevant to promtail. However this would create difficulties trying to copy/paste prometheus service discovery code into promtail.

@cyriltovena suggested silently omitting this and removing targets from the docs, which would keep compatibility while minimizing confusion in many cases.

@slim-bean Can you weigh in?

[adityacs]

@owen-d We don't have to name it dummy, we can make it localhost. But we should assign some target name otherwise target sync code will be skipped and file targets won't be initialized.

Also, I am completely for "removing this(targets) silently from the docs". Just that it should not confuse the existing users.

However this would create difficulties trying to copy/paste prometheus service discovery code into promtail.

Agree on this but I think users will eventually get adjusted to this

[slim-bean]

Wanted to get this into the 1.5 RC so I inserted my opinions via git push :)

I think using localhost instead of dummy will be more intuitive for people and less confusing/alarming if they are looking at the promtail /targets and /service-discovery http pages.

I also re-worded the docs a bit.

[slim-bean]

I ran this locally with and without targets and it LGTM!

[adityacs]

Thank you @slim-bean