Skip to content

Commit

Permalink
[logging] Clean up logging metadata abstractions (#20092)
Browse files Browse the repository at this point in the history
## Summary & Motivation

Clean up abstractions used in `DagsterLogManager`. The existing
abstractions were more confusing than necessary, with a mix of
dictionaries and NamedTuples, inconsistent proxied property access to
DagsterEvent, and methods/properties that were called only once.

- `DagsterLoggingMetadata` renamed to `DagsterLogHandlerMetadata`. This
is because there are multiple forms of "logging metadata" in the
system-- this item is specifically attached to the log manager and
merged into the metadata payload for each log record.
- `DagsterLogHandlerMetadata` is now a `TypedDict` instead of
`NamedTuple`. It needed to be converted to a dict downstream anyway, and
the presence of property fields on this was confusing, as they each had
only single callsites. It is simpler to make it a dumb dictionary (which
matches `DagsterLogRecordMetadata`) and compute what is needed on site.
- New `DagsterLogRecordMetadata` `TypedDict` to represent the full
payload attached to a log record after passing through the
`DagsterLogHandler`.
- Deleted `DagsterMessageProps`, which was a single-use class that just
gets merged into `DagsterLogRecord`.
- A few variable/property renames for clarity.

## How I Tested These Changes

Existing tests. Some tests were slightly updated, but just in internal
logic rather than end result, with one exception-- see inline comment on
that test.
  • Loading branch information
smackesey authored Feb 27, 2024
1 parent a5e6aa5 commit 154b1db
Show file tree
Hide file tree
Showing 5 changed files with 170 additions and 239 deletions.
18 changes: 9 additions & 9 deletions python_modules/dagster/dagster/_core/events/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,15 +162,15 @@ class DagsterEventType(str, Enum):
LOGS_CAPTURED = "LOGS_CAPTURED"


EVENT_TYPE_VALUE_TO_DISPLAY_STRING = {
"PIPELINE_ENQUEUED": "RUN_ENQUEUED",
"PIPELINE_DEQUEUED": "RUN_DEQUEUED",
"PIPELINE_STARTING": "RUN_STARTING",
"PIPELINE_START": "RUN_START",
"PIPELINE_SUCCESS": "RUN_SUCCESS",
"PIPELINE_FAILURE": "RUN_FAILURE",
"PIPELINE_CANCELING": "RUN_CANCELING",
"PIPELINE_CANCELED": "RUN_CANCELED",
EVENT_TYPE_TO_DISPLAY_STRING = {
DagsterEventType.PIPELINE_ENQUEUED: "RUN_ENQUEUED",
DagsterEventType.PIPELINE_DEQUEUED: "RUN_DEQUEUED",
DagsterEventType.PIPELINE_STARTING: "RUN_STARTING",
DagsterEventType.PIPELINE_START: "RUN_START",
DagsterEventType.PIPELINE_SUCCESS: "RUN_SUCCESS",
DagsterEventType.PIPELINE_FAILURE: "RUN_FAILURE",
DagsterEventType.PIPELINE_CANCELING: "RUN_CANCELING",
DagsterEventType.PIPELINE_CANCELED: "RUN_CANCELED",
}

STEP_EVENTS = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,11 +152,11 @@ def log(self) -> DagsterLogManager:

@property
def logging_tags(self) -> Mapping[str, str]:
return self.log.logging_metadata.all_tags()
return {k: str(v) for k, v in self.log.metadata.items()}

@property
def event_tags(self) -> Mapping[str, str]:
return self.log.logging_metadata.event_tags()
return {k: str(v) for k, v in self.log.metadata.items() if k != "job_tags"}

def has_tag(self, key: str) -> bool:
check.str_param(key, "key")
Expand Down
Loading

0 comments on commit 154b1db

Please sign in to comment.