chore: Fix promtail local config

[LashaJini]

What this PR does / why we need it:

👋, According to Build and run from source promtail should start without issues when only config file is passed (without passing any additional flags). However, because of the missing fields in the promtail-local-config.yaml file, it fails.

Checklist

• Reviewed the CONTRIBUTING.md guide (required)
• Documentation added
• Tests updated
• Title matches the required conventional commits format, see here
  • Note that Promtail is considered to be feature complete, and future development for logs collection will be in Grafana Alloy. As such, feat PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.
• Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
• For Helm chart changes bump the Helm chart version in production/helm/loki/Chart.yaml and update production/helm/loki/CHANGELOG.md and production/helm/loki/README.md. Example PR
• If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

[CLAassistant]

All committers have signed the CLA.

[LashaJini]

https://github.com/grafana/loki/blob/main/clients/cmd/promtail/main.go#L106

[LashaJini]

https://github.com/grafana/loki/blob/main/clients/pkg/promtail/positions/positions.go#L154

[LashaJini]

https://github.com/grafana/loki/blob/main/clients/pkg/promtail/targets/file/filetarget.go#L210

[trevorwhitney]

Thanks for the contribution. Can you share the error you recieved? I was able to start up without problem using this config locally. Some of the configs you're setting we have defaults for, for example log level

[LashaJini]

Thanks for the contribution. Can you share the error you recieved? I was able to start up without problem using this config locally. Some of the configs you're setting we have defaults for, for example log level

with current config:

After passing the log.level flag:

According to docs, I just assumed that passing the config file only, should be sufficient to start promtail.

[trevorwhitney]

you're using two different config paths in that example. cmd/promtail/promtail-local-config.yaml in the first screenshot should be clients/cmd/promtail/promtail-local-config.yaml

[LashaJini]

you're using two different config paths in that example. cmd/promtail/promtail-local-config.yaml in the first screenshot should be clients/cmd/promtail/promtail-local-config.yaml

My bad, wrong screenshot. I've added the correct one 👍

There is no promtail config in cmd directory, even though the docs states so.

[trevorwhitney]

Got it, thanks for the update, I was able to re-create. Thanks for offering a fix, but I think we should fix the code instead of just adding a band-aid to the config file. I spent a little time looking at this and I'm not sure why the level config is empty when we check it. We are definitely calling .RegisterFlags on it, which sets the level to info, so this seems like a regression to me.

[LashaJini]

Let me know if we should close the PR and open a new one. Meanwhile I can also look into it later 👍

[cstyan]

Got it, thanks for the update, I was able to re-create. Thanks for offering a fix, but I think we should fix the code instead of just adding a band-aid to the config file. I spent a little time looking at this and I'm not sure why the level config is empty when we check it. We are definitely calling .RegisterFlags on it, which sets the level to info, so this seems like a regression to me.

I was thinking the same recently, I think maybe https://github.com/grafana/loki/pull/13719/files is the cause of this as it might have changed how default values for some config structs are applied.

[LashaJini]

Got it, thanks for the update, I was able to re-create. Thanks for offering a fix, but I think we should fix the code instead of just adding a band-aid to the config file. I spent a little time looking at this and I'm not sure why the level config is empty when we check it. We are definitely calling .RegisterFlags on it, which sets the level to info, so this seems like a regression to me.

I was thinking the same recently, I think maybe https://github.com/grafana/loki/pull/13719/files is the cause of this as it might have changed how default values for some config structs are applied.

Just tested it. Commented out the whole UnmarshalYAML method, rebuilt promtail and works without any modification to the original config file 👍. but why 😃

[trevorwhitney]

@fgouteroux looks like #13719 may have introduced a regression here, thoughts on a fix?

[fgouteroux]

@fgouteroux looks like #13719 may have introduced a regression here, thoughts on a fix?

I think its is caused by: https://github.com/grafana/loki/blob/main/clients/pkg/promtail/config/config.go#L45 this is why there is default config var in https://github.com/prometheus/prometheus/blob/main/config/config.go#L146.

That explain why any default value from flag are not kept after UnmarshalYAML func.

[cstyan]

@fgouteroux looks like #13719 may have introduced a regression here, thoughts on a fix?

I think its is caused by: https://github.com/grafana/loki/blob/main/clients/pkg/promtail/config/config.go#L45 this is why there is default config var in https://github.com/prometheus/prometheus/blob/main/config/config.go#L146.

That explain why any default value from flag are not kept after UnmarshalYAML func.

The second link you gave was to Prometheus code, so it's not relevant to the issue reported here. For Loki and Promtail we use default values via our CLI arguments, so there were default values that were being overwritten by the UnmarshalYAML function. #14408 fixes the proble, so this PR shouldn't be necessary anymore.

@LashaJini let us know if you still see any similar problems.

[fgouteroux]

oh I miss this line 😂

[LashaJini]

@fgouteroux looks like #13719 may have introduced a regression here, thoughts on a fix?

I think its is caused by: https://github.com/grafana/loki/blob/main/clients/pkg/promtail/config/config.go#L45 this is why there is default config var in https://github.com/prometheus/prometheus/blob/main/config/config.go#L146.
That explain why any default value from flag are not kept after UnmarshalYAML func.

The second link you gave was to Prometheus code, so it's not relevant to the issue reported here. For Loki and Promtail we use default values via our CLI arguments, so there were default values that were being overwritten by the UnmarshalYAML function. #14408 fixes the proble, so this PR shouldn't be necessary anymore.

@LashaJini let us know if you still see any similar problems.

Awesome. Then it's just a matter of doc update. We can merge now 👍