Allow inline configuration vs file paths - #46
Conversation
91444db to
31faa1f
Compare
derrickstolee
left a comment
There was a problem hiding this comment.
I like this idea! I've taken a look and things seem reasonable.
It's worth noting that this is backwards-compatible with existing config files which is important. However, it is not backwards-compatible for compiling with consumers. Consumers may need to make a code update now that the data structures have changed.
| summaryContent := ` | ||
| message_patterns: | ||
| - prefix: "error:" | ||
| field_name: "error_count" | ||
|
|
||
| region_timers: | ||
| - category: "index" | ||
| label: "do_read_index" | ||
| time_field: "index_read_time" | ||
| ` |
There was a problem hiding this comment.
these kinds of yaml-string parsing tests are important, and my AI agent is warning that some of the parsing code might be fragile without explicit pairing of config member and yaml text. I know that we don't want this "summary path" option to be the main test, but this summary content should be tested for parsing in the inline way.
There was a problem hiding this comment.
Good call. I'll update Test_Config_Validate_WithAllOptionalSettings to retain raw YAML text for pii, filter, and summary. We can parse that YAML, explicitly pairing each member with its corresponding config field, and verify the resulting typed values after validation.
This will also mean exercising the inline object parsing, including the message_patterns, field_name, and region_timers mappings rather than making the legacy file path form the main test.
d12be64 to
3c4a160
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Collector decoding cannot populate the private raw fields, and inline important-event validation is incomplete.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Refactors receiver configuration to support inline PII, filter, and summary settings while retaining file-based configuration.
Changes:
- Adds inline configuration decoding and validation.
- Updates runtime consumers and tests to use typed settings.
- Revises documentation and examples for both configuration styles.
File summaries
| File | Description |
|---|---|
config.go |
Introduces inline/file configuration resolution. |
config_test.go |
Updates configuration validation tests. |
factory.go |
Removes obsolete path defaults. |
filter_settings.go |
Extracts filter validation. |
important_events_test.go |
Uses exported filter and summary fields. |
platform_unix.go |
Reads typed PII settings on Unix. |
platform_windows.go |
Reads typed PII settings on Windows. |
rcvr_base.go |
Logs enabled typed PII settings. |
summary.go |
Uses exported summary settings. |
summary_settings.go |
Extracts summary validation. |
summary_test.go |
Uses exported configuration fields. |
trace2dataset.go |
Uses typed filter and summary settings. |
Docs/config-filter-settings.md |
Documents inline filter configuration. |
Docs/config-pii-settings.md |
Documents inline PII configuration. |
Docs/configure-custom-collector.md |
Documents receiver configuration options. |
Docs/Examples/DebugDump/config.yml |
Demonstrates inline settings. |
Docs/Examples/ExportToAzureMonitor/config.yml |
Demonstrates inline settings. |
Review details
- Files reviewed: 17/17 changed files
- Comments generated: 6
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Separate settings files prevent using a single Collector configuration. Replace the pathname fields with typed PII, filter, and summary settings that the Collector can populate directly. Update runtime consumers and tests in the same commit so the tree remains buildable. Keep validation on the settings types so inline configuration receives the same checks as parsed YAML files. Assisted-by: Claude Opus 4.6 Assisted-by: GPT-5.6 Sol Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Describe PII, filter, and summary settings as inline receiver
configuration and update the example collectors to use that form.
Document ${file:PATH} as the external-file option and distinguish the
receiver-level wrapper from the contents expected in standalone PII
and filter files.
Assisted-by: Claude Opus 4.6
Assisted-by: GPT-5.6 Sol
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Existing deployments may provide bare file paths for PII, filter,
and summary settings. Preserve that syntax while continuing to accept
inline objects and Collector-expanded ${file:PATH} values.
Use exported raw carrier fields so the Collector decoder can populate
the string-or-object values, then resolve them into typed runtime
settings during validation. Exercise complete receiver stanzas through
confmap for both inline objects and legacy paths.
Assisted-by: Claude Opus 4.6
Assisted-by: GPT-5.6 Sol
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
Explain that PII, filter, and summary settings continue to accept bare file paths for existing deployments. Show the complete receiver example and clarify that standalone files omit their receiver-level wrapper keys. Assisted-by: Claude Opus 4.6 Assisted-by: GPT-5.6 Sol Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
3adda66 to
620258f
Compare
|
@derrickstolee please could I have another review of this PR please? I've restructured the commits and made a few edits:
Hopefully this addresses concerns Copilot and your AI Agent had. |
I’ll base the draft on the rewritten four-commit series and the final diff against
main, including the compatibility behavior and review-driven fixes.Summary
Allow the
pii,filter, andsummarysettings to be defined inline in the main OpenTelemetry Collector configuration. This makes it possible to configuretrace2receiverusing a single YAML file instead of requiring separate settings files.Each setting now accepts:
${file:PATH}value.The receiver decodes the string-or-object values into exported raw carrier fields, then resolves them into typed
PiiSettings,FilterSettings, andSummarySettingsduring validation.Validation
Move validation onto the settings types so it is shared by inline and file-backed configuration. In particular,
important_eventsrules receive the same required-field and duplicate-field checks regardless of how the filter settings were supplied.Compatibility
Existing Collector YAML files using plain PII, filter, or summary path strings continue to work.
The Go configuration API has changed. Consumers that construct
Configvalues directly must use the new typedPii,Filter, andSummaryfields, or the correspondingRawPii,RawFilter, andRawSummarycarrier fields when exercising configuration decoding.Documentation
Update the configuration reference and example collectors to show:
${file:PATH}.Testing
Add coverage that loads complete receiver stanzas through the Collector’s
confmapdecoder and verifies both inline objects and plain file-path strings. Existing parser and validation tests continue to cover malformed files, missing files, summary rules, filter rules, andimportant_events.