[runtime] Support per-event-type configurable log levels for event log#609
[runtime] Support per-event-type configurable log levels for event log#609weiqingy wants to merge 8 commits intoapache:mainfrom
Conversation
- Create EventLogLevel enum (OFF, STANDARD, VERBOSE) with case-insensitive fromString() parser in api module - Add 4 new config options to AgentConfigOptions: event-log.level, event-log.standard.max-string-length, event-log.standard.max-array-elements, event-log.standard.max-depth - Delete EventFilter.java — subsumed by the log level system - Remove eventFilter field and references from EventLoggerConfig and FileEventLogger - Remove 6 EventFilter-related tests from FileEventLoggerTest Part of apache#541: per-event-type configurable log levels.
- EventLogLevelResolver: resolves effective EventLogLevel for event type strings using hierarchical config key inheritance (exact match → parent package walk-up → root default → built-in STANDARD). Caches results in ConcurrentHashMap. Takes Map<String, Object> to avoid runtime→plan module dependency. - JsonTruncator: truncates Jackson JsonNode trees per configurable thresholds (max-string-length, max-array-elements, max-depth). Produces JSON-parseable wrapper objects (truncatedString, truncatedList, truncatedObject). Protected fields (eventType, id, attributes) at top level are never truncated. - Comprehensive unit tests for both classes covering exact match, hierarchy inheritance, edge cases, all truncation types, composition, and disabled thresholds. Part of apache#541: per-event-type configurable log levels.
…operator - EventLogRecord: add logLevel, eventType, truncator, and truncatedEventsCounter fields. Old 2-arg constructor defaults to VERBOSE for backward compatibility. - EventLogRecordJsonSerializer: write logLevel and eventType as top-level JSON fields. Apply JsonTruncator when level is STANDARD, increment truncatedEventsCounter when truncation occurs. - EventLogRecordJsonDeserializer: read optional logLevel field, default to VERBOSE for old records (backward compat). - FileEventLogger: resolve log level per event via EventLogLevelResolver, skip OFF events, initialize truncator from agent config. Accept truncation counter via setter. - ActionExecutionOperator: pass full AgentConfiguration.getConfData() to logger via agentConfig property. Wire truncation counter from BuiltInMetrics to FileEventLogger after open(). - BuiltInMetrics: add eventLogTruncatedEvents counter with getter. - FileEventLoggerTest: add 7 new tests for STANDARD truncation, VERBOSE preservation, OFF suppression, per-type override, hierarchical inheritance, new JSON fields, backward deserialization. Part of apache#541: per-event-type configurable log levels.
- Add EVENT_LOG_LEVEL, EVENT_LOG_MAX_STRING_LENGTH, EVENT_LOG_MAX_ARRAY_ELEMENTS, EVENT_LOG_MAX_DEPTH config options to Python AgentConfigOptions (keys and defaults match Java) - Update existing test_python_event_logging to verify new logLevel and top-level eventType fields in JSON output - Add test_event_log_verbose_level: verifies VERBOSE produces full content without truncation - Add test_event_log_off_level: verifies OFF suppresses all event logging - Add test_event_log_standard_truncation: verifies STANDARD with low max-string-length threshold produces truncatedString wrappers - Extract _run_event_logging_pipeline() and _read_log_records() helpers Part of apache#541: per-event-type configurable log levels.
- FileEventLogger: use PythonEvent.getEventType() (Python module path) for level resolution instead of EventContext.getEventType() (Java wrapper class name). Without this, per-type Python config keys like event-log.type.flink_agents.api.events.event.OutputEvent.level would never match. - EventLogRecordJsonSerializer: use Python module path for top-level eventType field for PythonEvents, consistent with the eventType inside the event object. - BuiltInMetrics: remove dead markEventTruncated() method — counter is incremented via serializer path directly. Part of apache#541: per-event-type configurable log levels.
|
Hey @wenjin272 @xintongsong could you please take a look at the PR? Feedback is welcome. Thanks! |
| Map<String, Object> data = confData != null ? confData : Collections.emptyMap(); | ||
|
|
||
| // Parse root default | ||
| Object rootValue = data.get(ROOT_LEVEL_KEY); |
There was a problem hiding this comment.
Shall we just use EVENT_LOG_LEVEL.getKey() here?
There was a problem hiding this comment.
Good suggestion. Changed to ConfigOption<EventLogLevel> with EventLogLevel.STANDARD as the default. AgentConfiguration.get() already supports enum types via ConfigurationUtils.convertValue(), so this works end-to-end.
| if (pythonType != null) { | ||
| topLevelEventType = pythonType; | ||
| } | ||
| } |
There was a problem hiding this comment.
The logic for extracting the python event type is the same as in FileEventLogger. We can extract a common util function.
There was a problem hiding this comment.
Good catch. Extracted EventUtil.resolveEventType(Event, EventContext) and replaced the duplicated logic in both FileEventLogger.append() and EventLogRecordJsonSerializer.serialize().
| private final EventLogLevel logLevel; | ||
| private final String eventType; | ||
| private final transient JsonTruncator truncator; | ||
| private final transient Counter truncatedEventsCounter; |
There was a problem hiding this comment.
Maybe we can mark truncator and truncatedEventsCounter as @Nullable.
Additionally, these two members are actually required variables for the serialization of EventLogRecord, rather than part of its content. During deserialization, we have to set them to null. I wonder if they could be removed from within EventLogRecord itself.
There was a problem hiding this comment.
Added @Nullable on both fields.
Regarding removing them from EventLogRecord — I agree they are serialization plumbing rather than content. However, Jackson's custom serializer (EventLogRecordJsonSerializer) receives only the record object via serialize(EventLogRecord record, ...). Since the ObjectMapper is shared/static, there's no clean way to pass the truncator and counter to the serializer except through the record itself. The transient + @Nullable markers signal that these are not part of the logical content. Happy to explore alternatives if you have a preferred pattern in mind.
| * values are "OFF", "STANDARD", and "VERBOSE". Defaults to "STANDARD". | ||
| */ | ||
| public static final ConfigOption<String> EVENT_LOG_LEVEL = | ||
| new ConfigOption<>("event-log.level", String.class, "STANDARD"); |
There was a problem hiding this comment.
Maybe we can just use enum here
public static final ConfigOption<EventLogLevel> EVENT_LOG_LEVEL =
new ConfigOption<>("event-log.level", EventLogLevel.class, EventLogLevel.STANDARD);
There was a problem hiding this comment.
Updated the PR.
| this.rootDefault = | ||
| rootValue != null | ||
| ? EventLogLevel.fromString(rootValue.toString()) | ||
| : BUILT_IN_DEFAULT; |
There was a problem hiding this comment.
Shall we just use EVENT_LOG_LEVEL.getDefaultValue() here?
There was a problem hiding this comment.
Done. Replaced BUILT_IN_DEFAULT with AgentConfigOptions.EVENT_LOG_LEVEL.getDefaultValue(). Both removed constants are now derived from the single config option definition.
| return; | ||
| } | ||
| eventLogger.open(new EventLoggerOpenParams(runtimeContext)); | ||
| if (eventLogger instanceof FileEventLogger) { |
There was a problem hiding this comment.
Why is it necessary to specifically check the type of eventLogger here?
There was a problem hiding this comment.
The instanceof check is needed because setTruncatedEventsCounter() is specific to FileEventLogger — it's not on the EventLogger interface. The truncation counter is registered centrally in BuiltInMetrics (alongside numOfEventProcessed, numOfActionsExecuted, etc.) so all operator metrics live in one place under the same FlinkAgentsMetricGroupImpl hierarchy. The operator then passes the counter reference to FileEventLogger after open().
We considered adding the counter to the EventLogger interface or having the logger self-register, but both options would either leak Flink metrics concerns into the public API or break the centralized metrics pattern. The instanceof check keeps the interface clean while allowing FileEventLogger to participate in the operator's metrics.
Open to suggestions if you'd prefer a different approach here.
- Use ConfigOption<EventLogLevel> enum type instead of String - Derive root key and default from EVENT_LOG_LEVEL config constant - Add @nullable on transient truncator/counter fields in EventLogRecord - Extract EventUtil.resolveEventType() for PythonEvent type resolution
|
@wenjin272 Thanks for the review! I’ve addressed your comments - could you take another look? |
Linked issue: #541 , #552
Purpose of change
Add per-event-type configurable log levels (
OFF,STANDARD,VERBOSE) to the event log system, as designed in #552.Key changes:
EventLogLevelenum (OFF,STANDARD,VERBOSE) with hierarchical config key inheritance (event-log.type.<EVENT_TYPE>.level)STANDARDlevel, large content fields are truncated using configurable per-field thresholds (max-string-length,max-array-elements,max-depth) with JSON-parseable wrapper objects (truncatedString,truncatedList,truncatedObject)logLevelandeventTypefields added to JSON log record schema for downstream filteringEventFilterremoved — fully subsumed by the log level systemeventLogTruncatedEventscounter metric for observabilityConfig options:
event-log.levelSTANDARDevent-log.type.<EVENT_TYPE>.levelevent-log.standard.max-string-length2000event-log.standard.max-array-elements20event-log.standard.max-depth5Tests
EventLogLevelResolverTest— 11 tests: exact match, hierarchy inheritance, root/built-in defaults, case insensitivity, edge casesJsonTruncatorTest— 8 tests: string/array/depth truncation, wrappers, protected fields, composition, disabled thresholdsFileEventLoggerTest— 7 new tests: STANDARD truncation, VERBOSE preservation, OFF suppression, per-type override, hierarchical inheritance, JSON schema fields, backward-compatible deserializationAPI
EventLogLevelenum inapimoduleEventFilterinterface (0.x pre-release, no known users)EventLoggerConfig— removedeventFilterfield/builder methodDocumentation
doc-needed