Skip to content

Monitoring: Document the option to leave headers out of logs - #400

Merged
iniw merged 3 commits into
mainfrom
vinicius/cor-1961-make-request-header-logging-optional-enabled-by-default
Oct 1, 2026
Merged

iniw merged 3 commits into
mainfrom
vinicius/cor-1961-make-request-header-logging-optional-enabled-by-default

Conversation

@iniw

@iniw iniw commented Oct 1, 2026

Copy link
Copy Markdown
Member

For COR-1961, operator change: metalbear-co/operator#2551.

The Message Processing logs write all HTTP request headers and all queue message properties, and these can contain secrets. The operator chart gets the operator.logHeadersAndProperties value. When it is false, the logs do not have the request_headers and message_properties fields, but they still have the trace and correlation fields.

The monitoring page lists these two fields as always present, and its warning about sensitive values only suggests redaction in the log collector. This updates the two field rows, and adds the option to the warning, so that users who read about the risk also find the way to turn it off.

Merge this only after the operator release that has the change.

#399 (COR-1962) also changes the same warning on this page, so the second of the two PRs to merge will need a small rebase.

The operator gets the `operator.logHeadersAndProperties` Helm value.
When it is `false`, the `Message Processing` logs do not have the
`request_headers` and `message_properties` fields. Tell users about
it in the field tables and in the warning about sensitive values.
@linear-code

linear-code Bot commented Oct 1, 2026

Copy link
Copy Markdown

COR-1961

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can keep summaries lean with Findings visible per group, which tucks the rest behind a View link

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@greptile-apps

greptile-apps Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Retrigger

[Low risk] Documents an existing logging configuration option.

The PR appears safe to merge after the stated Operator release, with two non-blocking documentation clarifications.

Fix All in CursorFindings

  1. P2 Missing supported version ▶
  2. P2 Conditional fields described as guaranteed ▶
Fix with agent prompt
### Issue 1
docs/managing-mirrord/monitoring.md:181
The new Helm setting has no minimum Operator or chart version. If an administrator uses an older installation, they could follow this warning and assume full headers and properties are no longer logged without knowing whether their installation supports the setting. State the first supported version so they can verify the mitigation before relying on it.

### Issue 2
docs/managing-mirrord/monitoring.md:181
The statement that records “still have” all four correlation and tracing fields implies they appear on every record. These fields depend on available headers or broker metadata, and the documented HTTP response record contains none of them. Say that disabling the full maps does not suppress these fields *when available*, so readers do not expect fields that may be absent.

```suggestion
To keep the full header and property maps out of these records, set `operator.logHeadersAndProperties` to `false` in the Operator Helm chart values. The records then do not have the `request_headers` and `message_properties` fields, but can still have the `correlation_id`, `traceparent`, `tracestate`, and `baggage` fields when the corresponding metadata is available.
```

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Summary

The PR documents a Helm option for omitting full HTTP header and message-property maps from Message Processing logs while retaining available correlation and tracing metadata.

  • Updates both field descriptions and the sensitive-metadata warning.
  • Greptile automatically discovered a related ticket that helped explain the purpose of this PR: make request-header logging optional while leaving it enabled by default.

Reviews (1) · Last reviewed commit: "Monitoring: Document the option to leave..."

Comment thread docs/managing-mirrord/monitoring.md Outdated
Comment thread docs/managing-mirrord/monitoring.md Outdated
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Document optional header and message-property logging

📝 Documentation 🕐 Less than 10 minutes

Grey Divider

AI Description

• Clarifies when HTTP header and queue message-property fields are absent from Message Processing
 logs.
• Documents the Helm setting for excluding those maps while retaining available correlation and
 tracing fields.
Diagram

graph TD
  A["Helm setting"] --> B{"Log maps?"} -->|true| C["Include maps"] --> E["Processing logs"]
  B -->|false| D["Omit maps"] --> E
  F["Available trace fields"] --> E
Loading
High-Level Assessment

Documenting the condition in both field tables and the sensitive-data warning makes it discoverable where readers need it. Merge only after the operator release supports the Helm value.

Files changed (1) +4 / -2

Documentation (1) +4 / -2
monitoring.mdExplain how to omit headers and properties from logs +4/-2

Explain how to omit headers and properties from logs

• Marks request_headers and message_properties as absent when operator.logHeadersAndProperties is false. Adds the Helm setting to the sensitive-data warning and clarifies that available correlation and tracing fields remain in the records.

docs/managing-mirrord/monitoring.md

@iniw
iniw requested a review from meowjesty October 1, 2026 12:13

@meowjesty meowjesty left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Phrasing can be better, it's a bit confusing.

Comment thread docs/managing-mirrord/monitoring.md Outdated
iniw added 2 commits October 1, 2026 15:06
The operator change flipped the option. It is now
`operator.hideHeadersAndProperties`, with the default `false`.
@iniw
iniw requested a review from meowjesty October 1, 2026 20:21
@iniw
iniw merged commit 3d06545 into main Oct 1, 2026
6 checks passed
@iniw
iniw deleted the vinicius/cor-1961-make-request-header-logging-optional-enabled-by-default branch October 1, 2026 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants