Vendor macOS Unified Logging receiver with reliable cursor tracking#74
Open
kroepke wants to merge 30 commits into
Open
Vendor macOS Unified Logging receiver with reliable cursor tracking#74kroepke wants to merge 30 commits into
kroepke wants to merge 30 commits into
Conversation
Extracts exec.Command usage behind a logRunner interface with an execLogRunner darwin implementation. runLogCommand now obtains stdout via r.runner.Run() and logs captured stderr on error. A temporary verifyLogBinary stub (integrity_darwin.go) lets execLogRunner compile; Task 11 replaces it with real integrity checks. No behavior change.
Guard fakeRunner.calls with a callCount() accessor in the test wait-loop, and join the poll goroutine in Shutdown via a WaitGroup so it cannot read shared config while a concurrent Validate() mutates it (caught by go test -race).
Adds receiver_integration_darwin_test.go (build tag: darwin && integration) that injects a unique marker via /usr/bin/logger, runs the real execLogRunner with a process-scoped predicate, and asserts the marker is emitted exactly once across multiple polls - proving live cursor + dedup end-to-end. Predicate uses processImagePath == "/usr/bin/logger" to exclude the macOS unified log self-recording of each `log show` invocation (which would otherwise contain the marker string and pollute the count).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
9 tasks
if the predicate changes, the stored cursor becomes meaningless and we have to start over one caveat is that we don't know _how_ a predicate changed, so we start at the max_age again, potentially reading duplicate values. solving that would require more server-side logic for the max_age etc, so we play it safe
when the collector wasn't running for longer than max_age, we now start where we left off, instead of clamping it to max_age split the cursor handling in two stages, only advancing it after ConsumeLogs has successfully returned (which means downstream is responsible for durability)
There was a problem hiding this comment.
Pull request overview
This PR vendors the upstream macOS Unified Logging receiver into this repository and wires it into the local build/test tooling, with additional live-mode correctness improvements (cursor persistence + boundary-second dedup) and platform integrity checks for /usr/bin/log.
Changes:
- Added a new
receiver/macosunifiedloggingreceiverGo module implementing the macOS unified logging receiver (live + archive modes), including cursor persistence and extensive unit/integration tests. - Integrated the new receiver into the repo Taskfiles and the collector builder configuration/components list.
- Updated top-level/module dependencies (
go.mod/go.sum) to support the new receiver and align OTel component versions.
Reviewed changes
Copilot reviewed 41 out of 47 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| Taskfile.yml | Adds the macOS unified logging receiver module to formatting/test/vulncheck orchestration. |
| receiver/macosunifiedloggingreceiver/testdata/test_config.yaml | Adds YAML configs used by receiver config-loading tests. |
| receiver/macosunifiedloggingreceiver/Taskfile.yml | Adds module-local go test task. |
| receiver/macosunifiedloggingreceiver/storage.go | Storage client resolution helper for persisting the live-mode cursor. |
| receiver/macosunifiedloggingreceiver/storage_test.go | Tests for storage client resolution and test storage/host fakes. |
| receiver/macosunifiedloggingreceiver/severity.go | Maps macOS messageType to OTel severity numbers. |
| receiver/macosunifiedloggingreceiver/severity_test.go | Unit tests for severity mapping. |
| receiver/macosunifiedloggingreceiver/receiver.go | Core receiver implementation: polling, parsing, batching, cursoring, persistence. |
| receiver/macosunifiedloggingreceiver/receiver_test.go | Unit tests covering dedup, cursor behavior, error handling, oversized lines, etc. |
| receiver/macosunifiedloggingreceiver/receiver_integration_darwin_test.go | Darwin-only integration test against real unified logging (logger + cursor dedup). |
| receiver/macosunifiedloggingreceiver/README.md | Receiver documentation, config options, security model, and behavior notes. |
| receiver/macosunifiedloggingreceiver/parser.go | NDJSON parsing and OTel log record mapping. |
| receiver/macosunifiedloggingreceiver/parser_test.go | Parser/mapping unit tests. |
| receiver/macosunifiedloggingreceiver/metadata.yaml | mdatagen metadata for receiver type/status/tests config. |
| receiver/macosunifiedloggingreceiver/logrunner.go | Runner interface abstraction for invoking log. |
| receiver/macosunifiedloggingreceiver/logrunner_darwin.go | Darwin implementation of log runner using integrity-verified /usr/bin/log. |
| receiver/macosunifiedloggingreceiver/internal/metadata/generated_status.go | Generated component status metadata. |
| receiver/macosunifiedloggingreceiver/internal/metadata/generated_logs.go | Generated logs builder for emitted logs. |
| receiver/macosunifiedloggingreceiver/internal/metadata/generated_logs_test.go | Generated tests for logs builder behavior. |
| receiver/macosunifiedloggingreceiver/integrity_darwin.go | /usr/bin/log (and optionally codesign) integrity verification. |
| receiver/macosunifiedloggingreceiver/integrity_darwin_test.go | Darwin-only tests for integrity checks. |
| receiver/macosunifiedloggingreceiver/go.sum | Dependency lockfile for the receiver module. |
| receiver/macosunifiedloggingreceiver/go.mod | New receiver module definition and dependencies. |
| receiver/macosunifiedloggingreceiver/generated_package_test.go | Generated goleak TestMain for the receiver package. |
| receiver/macosunifiedloggingreceiver/generated_component_test.go | Generated factory/config/lifecycle tests from mdatagen. |
| receiver/macosunifiedloggingreceiver/factory.go | Factory entrypoint and default config construction. |
| receiver/macosunifiedloggingreceiver/factory_others.go | Non-darwin factory returning unsupported-platform error. |
| receiver/macosunifiedloggingreceiver/factory_others_test.go | Tests for non-darwin factory behavior. |
| receiver/macosunifiedloggingreceiver/factory_darwin.go | Darwin factory creating the real receiver with exec runner + config validation. |
| receiver/macosunifiedloggingreceiver/factory_darwin_test.go | Tests for default config values on darwin. |
| receiver/macosunifiedloggingreceiver/doc.go | Package docs + mdatagen directive. |
| receiver/macosunifiedloggingreceiver/cursor.go | Cursor implementation (boundary-second dedup + predicate hash + persistence). |
| receiver/macosunifiedloggingreceiver/cursor_test.go | Cursor unit tests (dedup, reboot reset, idle poll, round-trip). |
| receiver/macosunifiedloggingreceiver/config.schema.yaml | Schema for receiver configuration (used by tooling/docs). |
| receiver/macosunifiedloggingreceiver/config.go | Darwin-only config validation, predicate validation, archive glob resolution. |
| receiver/macosunifiedloggingreceiver/config_test.go | Darwin-only tests for config validation and archive glob resolution. |
| receiver/macosunifiedloggingreceiver/config_defaults_test.go | Tests for default config values. |
| receiver/macosunifiedloggingreceiver/config_common.go | Platform-neutral config struct shared by factories and docs. |
| receiver/macosunifiedloggingreceiver/cadence.go | Live polling backoff cadence implementation. |
| receiver/macosunifiedloggingreceiver/cadence_test.go | Unit tests for cadence backoff/reset behavior. |
| go.sum | Updates root dependency lockfile. |
| go.mod | Updates root module dependencies (incl. added replaces/indirects). |
| changelog/unreleased/pr-TBD-macos-unified-logging-live-mode.toml | Adds an unreleased changelog entry describing the new receiver. |
| builder/go.mod | Adds local receiver module + adjusts OTel contrib dependency versions. |
| builder/components.go | Switches builder wiring to use the local macOS receiver module. |
| builder/builder-config.yaml | Updates builder config to reference the local receiver module + replace entry. |
Files not reviewed (3)
- builder/components.go: Generated file
- receiver/macosunifiedloggingreceiver/generated_component_test.go: Generated file
- receiver/macosunifiedloggingreceiver/generated_package_test.go: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+62
to
+69
| func (r *unifiedLoggingReceiver) Start(_ context.Context, host component.Host) error { | ||
| if r.cfg.ArchivePath == "" { | ||
| client, err := getStorageClient(context.Background(), host, r.cfg.StorageID, r.id) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| r.storage = client | ||
| if data, err := client.Get(context.Background(), cursorStorageKey); err == nil && len(data) > 0 { |
Comment on lines
+21
to
+26
| predicate: | ||
| description: 'Predicate is a filter predicate to pass to the log command Example: "subsystem == ''com.apple.systempreferences''"' | ||
| type: string | ||
| start_time: | ||
| description: 'StartTime specifies when to start reading logs from Format: "2006-01-02 15:04:05"' | ||
| type: string |
| - Redirects: `>>`, `<<` | ||
| - Control characters: newlines, carriage returns | ||
|
|
||
| Valid predicate operators like `&&` (logical AND), `<`, `>` (comparison) are allowed. The `>` operator is allowed for comparisons (e.g., `processID > 100`) but blocked when followed by file paths. Note that `&&` is automatically normalized to `AND` for consistency. Use standard predicate syntax as documented by Apple's `log` command. |
Comment on lines
+23
to
+30
| ext, ok := host.GetExtensions()[*storageID] | ||
| if !ok { | ||
| return nil, fmt.Errorf("storage extension %q not found", storageID) | ||
| } | ||
| se, ok := ext.(storage.Extension) | ||
| if !ok { | ||
| return nil, fmt.Errorf("non-storage extension %q configured as storage", storageID) | ||
| } |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 47 changed files in this pull request and generated 6 comments.
Files not reviewed (3)
- builder/components.go: Generated file
- receiver/macosunifiedloggingreceiver/generated_component_test.go: Generated file
- receiver/macosunifiedloggingreceiver/generated_package_test.go: Generated file
Comment on lines
+24
to
+26
| if !ok { | ||
| return nil, fmt.Errorf("storage extension %q not found", storageID) | ||
| } |
Comment on lines
+27
to
+30
| se, ok := ext.(storage.Extension) | ||
| if !ok { | ||
| return nil, fmt.Errorf("non-storage extension %q configured as storage", storageID) | ||
| } |
Comment on lines
+43
to
+51
| "signpostType": true, | ||
| "size": true, | ||
| "subsystem": true, | ||
| "threadIdentifier": true, | ||
| "timeToLive": true, | ||
| "traceIdentifier": true, | ||
| "transitionActivityIdentifier": true, | ||
| "type": true, | ||
| } |
Comment on lines
+1
to
+26
| description: Config defines configuration for the macOS unified logging receiver Separated into a common file that isn't platform specific so that factory_others.go can reference it | ||
| type: object | ||
| properties: | ||
| archive_path: | ||
| description: ArchivePath is a path or glob pattern to .logarchive directory(ies) If empty, reads from the live system logs Supports glob patterns (e.g., "*.logarchive", "**/logs/*.logarchive") | ||
| type: string | ||
| end_time: | ||
| description: EndTime specifies when to stop reading logs Only used with archive_path | ||
| type: string | ||
| format: | ||
| description: 'Format specifies the output format from the log command Options: "default" (system default), "ndjson", "json", "syslog", "compact" Default: "default"' | ||
| type: string | ||
| max_log_age: | ||
| description: 'MaxLogAge specifies the maximum age of logs to read on startup Only applies to live mode. Format: "24h", "1h30m", etc.' | ||
| type: string | ||
| format: duration | ||
| max_poll_interval: | ||
| description: MaxPollInterval specifies the maximum interval between polling for new logs (live mode only) The actual poll interval uses exponential backoff based on whether logs are being actively written | ||
| type: string | ||
| format: duration | ||
| predicate: | ||
| description: 'Predicate is a filter predicate to pass to the log command Example: "subsystem == ''com.apple.systempreferences''"' | ||
| type: string | ||
| start_time: | ||
| description: 'StartTime specifies when to start reading logs from Format: "2006-01-02 15:04:05"' | ||
| type: string |
Comment on lines
+70
to
+75
| type fakeHost struct { | ||
| exts map[component.ID]component.Component | ||
| } | ||
|
|
||
| func (h fakeHost) GetExtensions() map[component.ID]component.Component { return h.exts } | ||
|
|
Comment on lines
+62
to
+83
| func (r *unifiedLoggingReceiver) Start(_ context.Context, host component.Host) error { | ||
| if r.cfg.ArchivePath == "" { | ||
| client, err := getStorageClient(context.Background(), host, r.cfg.StorageID, r.id) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| r.storage = client | ||
| if data, err := client.Get(context.Background(), cursorStorageKey); err == nil && len(data) > 0 { | ||
| switch c, lerr := loadCursor(data); { | ||
| case lerr != nil: | ||
| r.logger.Warn("could not load persisted cursor; starting fresh", zap.Error(lerr)) | ||
| case c.predicateHash != r.cursor.predicateHash: | ||
| r.logger.Info("predicate changed since last run; discarding persisted cursor and starting fresh", | ||
| zap.String("persisted", c.predicateHash), zap.String("current", r.cursor.predicateHash)) | ||
| default: | ||
| r.cursor = c | ||
| } | ||
| } | ||
| } | ||
|
|
||
| ctx, cancel := context.WithCancel(context.Background()) | ||
| r.cancel = cancel |
kroepke
changed the title
Member
|
@kroepke I noticed that the Should we use the |
Member
Author
I had initially thought that this is fine, since we are removing |
previously we used localtime, which would break on device tz changes and DST now everything is always UTC so we can reliably continue from the exact second unparseable timestamps cause the message to be ignored (but this should never happen in realistic scenarios)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Vendors macosunifiedloggingreceiver from opentelemetry-collector-contrib v0.153.0 (commit 42f9491) into this repo, replacing the upstream contrib module in the build — same pattern as the existing windowseventlogreceiver.
The upstream live mode re-emits boundary events on every poll and has no persistence; this fork makes live mode production-reliable.
Key Changes:
(machTimestamp, threadID)to be able to dedup on second boundaries, which is the granularity we can get data from the/usr/bin/logcommandbootUUIDand the predicate hash - changes invalidate the cursormin_poll_interval(1s default to avoidlog showamplify its own output) up tomax_poll_intervalndjsonoutput, message body iseventMessage, anything else is prefixed withmacos./usr/bin/logThe collector builds cross-platform, but the important tests are essentially no-ops.
Non-obvious detail: We only advance the cursor after
ConsumeLogssucceeds to make the backpressure inotel-collectorwork.