fix(instigation logger): stringify non-json objects

[jonaslb]

Summary & Motivation

Fixes #32845. Although, here is probably some underlying issue that is not fixed by this PR. But I think that can go in another issue.

How I Tested These Changes

Implemented a test with a "weird" log record to show that logging does not error in this case any more.

Changelog

InstigationLogger handles LogRecord attributes that are not json serializable by stringifying them instead of logging an error

[cmpadden]

I verified that this preserves the prior seven.json behavior.

One small caveat is that this won’t make serialization completely bulletproof for every possible pathological LogRecord shape, e.g. unusual nested dict keys or circular structures, but it’s an appropriate minimal fix for the issue reported in #32845.

[greptile-apps]

Greptile Summary

This PR fixes a crash in InstigationLogger where log records containing non-JSON-serializable objects (e.g., custom Python classes passed as extra kwargs) would silently fail and write an error to stderr instead of the log stream. The fix uses Python's standard json module with default=str to stringify unserializable values, and improves the fallback error message to include the originating call site.

• Replaces seven.json.dumps with json.dumps(..., default=str, sort_keys=True) so any non-serializable value is stringified rather than causing an exception; the except branch now also reports the origin file/line.
• Replaces seven.json.loads with json.loads(line, strict=False) on the read path, relaxing control-character handling.
• Adds a test that logs a custom non-serializable object and asserts no errors are written to stderr and the record is correctly stored.

Confidence Score: 4/5

Safe to merge; the core fix is correct and the new test exercises the repaired path end-to-end.

The default=str addition is the right fix for non-serializable log extras, the error fallback now includes useful origin context, and the test verifies the full round-trip. The two minor items — an unnecessary strict=False on the read path and a slightly loose record-count assertion in the test — do not affect correctness at runtime.

No files require special attention; both changed files are straightforward and the test covers the fixed scenario well.

Important Files Changed

Filename	Overview
python_modules/dagster/dagster/_core/definitions/instigation_logger.py	Core fix: replaces seven.json with stdlib json + default=str to handle non-serializable log record attributes; adds strict=False to the read path (unnecessary but harmless).
python_modules/dagster/dagster_tests/storage_tests/test_instigation_logger.py	New end-to-end test covers the happy path with a non-serializable extra; uses >= 2 for record count which is slightly imprecise but functionally adequate.

Sequence Diagram

sequenceDiagram
    participant User as User Code
    participant IL as InstigationLogger
    participant CLH as CapturedLogHandler
    participant WS as Write Stream (IO)
    participant GILR as get_instigation_log_records

    User->>IL: "logger.info("msg", extra={"key": NonJsonSerializable()})"
    IL->>IL: makeRecord() → _annotate_record()
    IL->>CLH: emit(record)
    CLH->>CLH: "record_dict = record.__dict__"
    CLH->>CLH: handle exc_info (format traceback string)
    alt "default=str path (NEW)"
        CLH->>CLH: "json.dumps(record_dict, default=str, sort_keys=True)"
        CLH->>WS: write(json_line + newline)
    else Exception still raised (circular ref, etc.)
        CLH->>CLH: except: write to stderr with origin location
    end
    Note over GILR,WS: Later, on read-back
    GILR->>WS: get_log_data() → raw bytes
    GILR->>GILR: for line in raw_logs.split(newline)
    GILR->>GILR: "json.loads(line, strict=False)"
    GILR-->>User: Sequence[Mapping[str, Any]]

Loading

_{Reviews (1): Last reviewed commit: "Remove unnecessary pyright ignore" | Re-trigger Greptile}

[greptile-apps]

strict=False is redundant on this read path. json.dumps always escapes control characters in strings (e.g., a newline in a str value becomes in the JSON output), so the data written by CapturedLogHandler.emit is already strictly conformant JSON and will parse without strict=False. The parameter adds no protection here and makes the code misleadingly imply that the stored JSON might contain raw control characters.

Suggested change

	records.append(json.loads(line, strict=False))
	records.append(json.loads(line))

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!

[greptile-apps]

The test logs exactly two messages and then asserts len(records) >= 2. If a future change accidentally swallowed one record (returning only 1), this assertion would still pass. An == 2 assertion (or checking that both messages appear) would catch that regression.

Suggested change

	assert len(records) >= 2
	assert "test-repo - test-instigator - log with extras" in {
	record.get("msg") for record in records if record.get("msg")
	}
	assert len(records) == 2
	record_msgs = {record.get("msg") for record in records if record.get("msg")}
	assert "test-repo - test-instigator - log without extras" in record_msgs
	assert "test-repo - test-instigator - log with extras" in record_msgs