Promtail: Add custom headers on push requests (#7470)

[Jack-Kingdom]

What this PR does / why we need it:
add custom headers support on promtail send reqeust

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

Checklist

• Reviewed the CONTRIBUTING.md guide
• Documentation added
• Tests updated
• CHANGELOG.md updated
• Changes that require user attention or interaction to upgrade are documented in docs/sources/upgrading/_index.md

[CLAassistant]

All committers have signed the CLA.

[grafanabot]

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
- querier/queryrange	-0.1%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

[chaudum]

Hey @Jack-Kingdom thanks for your PR. I proposed some minor changes.

[liguozhong]

clients:
  - name: custom-headers
    url: ../api/v1/push
    tenant_id: tenant1
    custom_headers:
       X-Scope-OrgID: tenant2

What will happen to custom_header if it has X-Scope-OrgID header, tenant1 or tenant2？
should we avoid user error and report error before promtail startup.for example

// UnmarshalYAML implement Yaml Unmarshaler
func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error {
	...
	if err := unmarshal(&cfg); err != nil {
		return err
	}

	//Validate config
	if c.TenantID!=""&&len(c.CustomHeaders)>0 {
		_,ok:=c.CustomHeaders["X-Scope-OrgID"]
		if ok {
			return errors.New("illegal promtail header X-Scope-OrgID")
		}
	}

	*c = Config(cfg)
	return nil
}

[chaudum]

What will happen to custom_header if it has X-Scope-OrgID header, tenant1 or tenant2？ should we avoid user error and report error before promtail startup.for example

In the current implementation X-Scope-OrgID in custom_headers would replace the header value set by tenant_id. However, it should be the other way round. tenant_id should have higher priority. @Jack-Kingdom There should be an explicit test for this case as well.

[grafanabot]

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

-           ingester	-0.1%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

[Jack-Kingdom]

IMO it's inelegant to check config race condition. I update those logical, so that makt it won't override existing headers.

	// Add custom headers on request
	if len(c.cfg.CustomHeaders) > 0 {
		for k, v := range c.cfg.CustomHeaders {
			if req.Header.Get(k) == "" {
				req.Header.Add(k, v)
			}else {
				level.Warn(c.logger).Log("msg", "custom header key already exists, skipping", "key", k)
			}
		}
	}

doc mentioned this

[grafanabot]

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

[liguozhong]

clients:
  - name: custom-headers
    custom_headers:
       X-Scope-OrgID: tenant2

clients:
  - name: headers
    headers:
       X-Scope-OrgID: tenant2

What do you think, if we replace custom_headers with headers, custom_headers is too long for me and it's easy for me to make mistakes.

https://prometheus.io/docs/prometheus/latest/configuration/configuration/#remote_write
prometheus doc

# Custom HTTP headers to be sent along with each remote write request.
# Be aware that headers that are set by Prometheus itself can't be overwritten.
headers:
  [ key: value ... ]

[liguozhong]

In addition, this header feature is great, we have some gateway systems for security, need to have some authentication header.

[MichelHollands]

@Jack-Kingdom Thanks for this contribution. This is looking good. There is an open question on renaming this to headers. This would make it consistent with Loki. Can you have a look at that?

[chaudum]

@Jack-Kingdom Do you want to address the remaining issue (renaming to headers)? Otherwise I will change it so we can merge the feature into the main branch.

[grafanabot]

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

[grafanabot]

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

[grafanabot]

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

[Jack-Kingdom]

@chaudum Hi，I have rename custom_header to header in client config. Please hava a review of this commit.

[chaudum]

Thanks for changes. I have two small wording suggestions for documentation.

[Jack-Kingdom]

@chaudum I made some changes to those codes and combine them into a single commit. Please take a review.

[jeschkies]

@Jack-Kingdom could you merge main and see that the build gets green. I can merge it then 😊

[JStickler]

[Docs squad] LGTM

[hatrena]

Hi, Is this already released?
I am using grafana/promtail:2.7.4 and getting this error:
field headers not found in type client.raw

[DylanGuedes]

Hi, Is this already released? I am using grafana/promtail:2.7.4 and getting this error: field headers not found in type client.raw

This wasn't added to v2.7.4 because it is a new feature instead of a fix/patch.