added error/warn to be captured for otel logs

[ehvs]

Which issue this PR addresses:

Fixes https://redhat.atlassian.net/browse/ARO-28583
Where the error logs from otel were being silenced dropped

What this PR does / why we need it:

Zap format matching in filter/keep-only-high-signal — The collector's own logs use zap's tab-separated format (\terror\t, \twarn\t), which didn't match any of the existing high-signal patterns (designed for klog and logrus). This meant collector error/warning messages (gateway refusals, parse failures, connection resets) were silently dropped and never ingested. The filter now includes a pattern for zap-formatted error and warn levels.

Test plan for issue:

Done. Results of the json body

		"raw_json_body": {
			"message": "[core] [Channel #1 SubChannel #2] grpc: addrConn.createTransport failed to connect to {Addr: \"10.0.0.5:4317\", ServerName: \"telemetry.eastus.aro.azure.com:4317\", }. Err: connection error: desc = \"transport: Error while dialing: dial tcp 10.0.0.5:4317: i/o timeout\"",
			"severity": "warn",
			"src_file": "grpc@v1.81.1/clientconn.go:1534",
			"fields": "{\"resource\": {\"service.instance.id\": \"<redacted>\", \"service.name\": \"telemetryexporter\", \"service.version\": \"1.25\"}, \"grpc_log\": true}"
		},
		"source_name": "containers",
		"subscription_id": "<redacted>",
		"resource_id": "<redacted>",
		"container": "otel-exporter",
		"region": "eastus",
		"log.iostream": "stderr",
		"namespace": "openshift-azure-logging",
		"EventName": "containers",
		"environment": "prod",
		"resource_name": "<redacted>",
		"logtag": "F",
		"pod": "otel-exporter-worker-ttdn8",
		"resource_group": "<redacted>"

Is there any documentation that needs to be updated for this PR?

No

How do you know this will function as expected in production?

Due to test plan.

[Copilot]

Pull request overview

Updates the OpenTelemetry Collector filter expression used by the ARO Operator geneva logging controller to ensure collector-originated zap-formatted error/warn logs are treated as “high-signal” and not dropped.

Changes:

• Extends the filter/keep-only-high-signal OTTL match logic to recognize zap’s tab-separated log level tokens (intended: \terror\t / \twarn\t).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[tuxerrante]

Thanks for chasing the missing collector errors here. I think this one needs a fix before merge.

The new zap matcher is being added after the existing IsString(body) and (...) branch has already been closed, so keep-only-high-signal-expr ends up unbalanced before the or (IsMap(body) ...) clause:

ARO-RP/pkg/operator/controllers/genevalogging/staticfiles/otel-config.yaml.tmpl

Lines 383 to 394 in 6c58b67

	{{- define "keep-only-high-signal-expr" -}}
	not ((IsString(body) and (
	IsMatch(body, "level=(error|warn|warning)")
	or IsMatch(body, "\\bE[0-9]{4}\\b|\\bW[0-9]{4}\\b")
	or IsMatch(body, "Update error .*: ClusterOperatorDegraded")))
	or IsMatch(body, "\\terror\\t|\\twarn\\t")))
	or (IsMap(body) and (
	(IsString(body["severity"]) and IsMatch(body["severity"], "^[EWF]$"))
	or (IsString(body["message"]) and (
	IsMatch(body["message"], "level=(error|warn|warning)")
	or IsMatch(body["message"], "\\bE[0-9]{4}\\b|\\bW[0-9]{4}\\b")
	or IsMatch(body["message"], "Update error .*: ClusterOperatorDegraded"))))))

That turns this from "we still miss some logs" into "we can ship a broken collector filter/config" on the minimal-logs path, which is a worse failure mode than the original bug in ARO-28583.

Could we move the new IsMatch(body, "\\terror\\t|\\twarn\\t") back inside the existing string-body branch and add focused coverage for this template/filter as part of the fix? A small test that renders/parses the minimal-logs config would go a long way here, because right now the operator only renders the template into a ConfigMap and the otel parser is the first layer that validates the OTTL.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

pkg/operator/controllers/genevalogging/staticfiles/otel-config.yaml.tmpl:388

• The keep-only-high-signal-expr boolean grouping looks incorrect: not currently only applies to the IsString(body) branch (not ((IsString(body) and ...)) or (IsMap(body) and ...)). For non-string bodies, IsString(body) is false so not(false) is true, which makes the whole expression true and causes the filter processor to drop all map-bodied log records regardless of the IsMap(body) sub-expression. If the intent is to drop logs that are not high-signal for both string and map bodies (consistent with other keep-* filters), wrap the entire OR in the not(...).

not ((IsString(body) and (
    IsMatch(body, "level=(error|warn|warning)")
    or IsMatch(body, "\\bE[0-9]{4}\\b|\\bW[0-9]{4}\\b")
    or IsMatch(body, "Update error .*: ClusterOperatorDegraded")
    or IsMatch(body, "\\terror\\t|\\twarn\\t")))

[tuxerrante]

I think the outer grouping is still off here. This now moves the zap matcher back inside the IsString(body) branch, but the outer not(...) still closes before the or (IsMap(body) ...) clause, so the expression reads like not(<string branch>) or <map branch> instead of not(<string branch> or <map branch>). Could we keep both branches inside the same outer not(...)?

[ehvs]

I took a second look, and the first ( at 384 closes at line 394. The second ( at 384 as well, closes on 388.
so the reading of the expression should be ok. the outer ( belongs to not, and the inner to IsString

[tuxerrante]

Approved based on the current head.

I re-checked the keep-only-high-signal grouping and it is correct as written now: the outer not(...) still spans both the string and map branches, while the new zap matcher stays inside the existing string-body clause. The added tests around rendered YAML, expression balance, and presence of the zap pattern are also a reasonable regression net for this change, and CI is green.

[tuxerrante]

@ehvs are you going to test "With ARO Operator in a cluster with WriteRequestBodies" before merging?

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[ehvs]

Upon testing I had to expand my changes.
What happened is that in Kusto it was showing the message with the entire raw line dumped into raw_json_body as a flat string instead of being parsed into the fields.
The cause is that our filelog receiver only had parsers for klog and logrus formats while Zap uses a tab-separated format that neither regex matched, so the body passed through unparsed and raw_json_body ended up as just the raw string. So I added a third parser (zap_parse) to the operator chain to handle it.