Skip to content

Commit 7de0f2c

Browse files
committed
Log which ops require each resource during initialization
The RESOURCE_INIT_STARTED log line previously listed only resource keys: Starting initialization of resources [io_manager, resource_a, resource_b]. On jobs with many resources this made it impossible to tell which op pulled in a given resource. It now attributes each resource to the ops that require it: Starting initialization of resources: io_manager - required by the I/O layer resource_a - required by op instances [op_one] resource_b - required by op instances [op_two] get_resource_origins_mapping builds resource_key -> {op instance names} from op and hook required_resource_keys, keyed by full node handle so nested and aliased ops are unambiguous, and propagates through resource-to-resource dependencies. job_def is threaded to the resource init generator to supply it; when absent (e.g. build_resources) the original message is preserved. The dagstermill resource manager is updated to match the new signature. Resolves #2307.
1 parent 7e8a130 commit 7de0f2c

5 files changed

Lines changed: 235 additions & 4 deletions

File tree

python_modules/dagster/dagster/_core/events/__init__.py

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,26 @@ def log_resource_event(log_manager: DagsterLogManager, event: "DagsterEvent") ->
410410
log_manager.log_dagster_event(level=log_level, msg=event.message or "", dagster_event=event)
411411

412412

413+
def _resource_init_start_message(
414+
resource_keys: AbstractSet[str],
415+
resource_key_to_op_names: Mapping[str, AbstractSet[str]] | None,
416+
) -> str:
417+
if not resource_key_to_op_names:
418+
return "Starting initialization of resources [{}].".format(", ".join(sorted(resource_keys)))
419+
420+
# Iterate resource_keys (scoped to this step) rather than the job-wide mapping, so a step
421+
# only lists its own ops. Keys not in the mapping are required by an io/input manager or
422+
# type loader rather than by an op directly.
423+
lines = ["Starting initialization of resources:"]
424+
for resource_key in sorted(resource_keys):
425+
op_names = sorted(resource_key_to_op_names.get(resource_key, set()))
426+
if op_names:
427+
lines.append(f"{resource_key} - required by op instances [{', '.join(op_names)}]")
428+
else:
429+
lines.append(f"{resource_key} - required by the I/O layer")
430+
return "\n".join(lines)
431+
432+
413433
class DagsterEventSerializer(NamedTupleSerializer["DagsterEvent"]):
414434
def before_unpack(self, context, unpacked_dict: Any) -> dict[str, Any]:
415435
event_type_value, event_specific_data = _handle_back_compat(
@@ -1337,15 +1357,14 @@ def resource_init_start(
13371357
execution_plan: "ExecutionPlan",
13381358
log_manager: DagsterLogManager,
13391359
resource_keys: AbstractSet[str],
1360+
resource_key_to_op_names: Mapping[str, AbstractSet[str]] | None = None,
13401361
) -> "DagsterEvent":
13411362
return DagsterEvent.from_resource(
13421363
DagsterEventType.RESOURCE_INIT_STARTED,
13431364
job_name=job_name,
13441365
execution_plan=execution_plan,
13451366
log_manager=log_manager,
1346-
message="Starting initialization of resources [{}].".format(
1347-
", ".join(sorted(resource_keys))
1348-
),
1367+
message=_resource_init_start_message(resource_keys, resource_key_to_op_names),
13491368
event_specific_data=EngineEventData(metadata={}, marker_start="resources"),
13501369
)
13511370

python_modules/dagster/dagster/_core/execution/context_creation_job.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,7 @@ def execution_context_event_generator(
238238
instance=instance,
239239
emit_persistent_events=True,
240240
event_loop=event_loop,
241+
job_def=job_def,
241242
)
242243
yield from resources_manager.generate_setup_events()
243244
scoped_resources_builder = check.inst(

python_modules/dagster/dagster/_core/execution/resources_init.py

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
import inspect
22
from asyncio import AbstractEventLoop
3-
from collections import deque
3+
from collections import defaultdict, deque
44
from collections.abc import Generator, Mapping
55
from contextlib import ContextDecorator
66
from typing import AbstractSet, Any, Callable, cast # noqa: UP035
77

88
from dagster_shared.utils.timing import format_duration
99

1010
import dagster._check as check
11+
from dagster._core.definitions.dependency import OpNode
1112
from dagster._core.definitions.job_definition import JobDefinition
1213
from dagster._core.definitions.resource_definition import (
1314
ResourceDefinition,
@@ -49,6 +50,7 @@ def resource_initialization_manager(
4950
instance: DagsterInstance | None,
5051
emit_persistent_events: bool | None,
5152
event_loop: AbstractEventLoop | None,
53+
job_def: JobDefinition | None = None,
5254
):
5355
generator = resource_initialization_event_generator(
5456
resource_defs=resource_defs,
@@ -60,6 +62,7 @@ def resource_initialization_manager(
6062
instance=instance,
6163
emit_persistent_events=emit_persistent_events,
6264
event_loop=event_loop,
65+
job_def=job_def,
6366
)
6467
return EventGenerationManager(generator, ScopedResourcesBuilder)
6568

@@ -116,6 +119,39 @@ def _get_deps_helper(resource_key):
116119
return reqd_resources
117120

118121

122+
def get_resource_origins_mapping(job_def: JobDefinition) -> Mapping[str, AbstractSet[str]]:
123+
"""Maps each resource key to the op instances that require it, keyed by full node handle so
124+
that aliased and nested ops are unambiguous.
125+
126+
A resource required by another resource (e.g. spark, pulled in by a pyspark resource) is
127+
attributed to the ops that required the outer resource. Resources no op requires directly,
128+
such as the default io_manager, are absent from the mapping.
129+
"""
130+
origins: dict[str, set[str]] = defaultdict(set)
131+
132+
for handle in job_def.graph.iterate_node_handles():
133+
node = job_def.get_node(handle)
134+
if not isinstance(node, OpNode):
135+
continue
136+
137+
op_name = str(handle)
138+
139+
for resource_key in node.definition.required_resource_keys:
140+
origins[resource_key].add(op_name)
141+
142+
for hook_def in job_def.get_all_hooks_for_handle(handle):
143+
for resource_key in hook_def.required_resource_keys:
144+
origins[resource_key].add(op_name)
145+
146+
resource_dependencies = resolve_resource_dependencies(job_def.resource_defs)
147+
for resource_key, op_names in list(origins.items()):
148+
for dep_key in get_dependencies(resource_key, resource_dependencies):
149+
if dep_key != resource_key:
150+
origins[dep_key].update(op_names)
151+
152+
return dict(origins)
153+
154+
119155
def _core_resource_initialization_event_generator(
120156
resource_defs: Mapping[str, ResourceDefinition],
121157
resource_configs: Mapping[str, ResourceConfig],
@@ -127,6 +163,7 @@ def _core_resource_initialization_event_generator(
127163
instance: DagsterInstance | None,
128164
emit_persistent_events: bool | None,
129165
event_loop,
166+
job_def: JobDefinition | None = None,
130167
):
131168
job_name = "" # Must be initialized to a string to satisfy typechecker
132169
contains_generator = False
@@ -147,6 +184,7 @@ def _core_resource_initialization_event_generator(
147184
cast("ExecutionPlan", execution_plan),
148185
resource_log_manager,
149186
resource_keys_to_init,
187+
get_resource_origins_mapping(job_def) if job_def else None,
150188
)
151189

152190
resource_dependencies = resolve_resource_dependencies(resource_defs)
@@ -225,6 +263,7 @@ def resource_initialization_event_generator(
225263
instance: DagsterInstance | None,
226264
emit_persistent_events: bool | None,
227265
event_loop: AbstractEventLoop | None,
266+
job_def: JobDefinition | None = None,
228267
):
229268
check.inst_param(log_manager, "log_manager", DagsterLogManager)
230269
resource_keys_to_init = check.opt_set_param(
@@ -233,6 +272,7 @@ def resource_initialization_event_generator(
233272
check.opt_inst_param(execution_plan, "execution_plan", ExecutionPlan)
234273
check.opt_inst_param(dagster_run, "dagster_run", DagsterRun)
235274
check.opt_inst_param(instance, "instance", DagsterInstance)
275+
check.opt_inst_param(job_def, "job_def", JobDefinition)
236276

237277
if execution_plan and execution_plan.step_handle_for_single_step_plans():
238278
step = execution_plan.get_step(
@@ -260,6 +300,7 @@ def resource_initialization_event_generator(
260300
instance=instance,
261301
emit_persistent_events=emit_persistent_events,
262302
event_loop=event_loop,
303+
job_def=job_def,
263304
)
264305
except GeneratorExit:
265306
# Shouldn't happen, but avoid runtime-exception in case this generator gets GC-ed

python_modules/dagster/dagster_tests/core_tests/test_job_init.py

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,13 @@
22
import pytest
33
from dagster import DagsterInstance
44
from dagster._core.definitions.job_base import InMemoryJob
5+
from dagster._core.events import DagsterEvent, DagsterEventType
6+
from dagster._core.execution import resources_init
57
from dagster._core.execution.api import create_execution_plan
68
from dagster._core.execution.context_creation_job import PlanExecutionContextManager
79
from dagster._core.execution.resources_init import (
10+
get_required_resource_keys_to_init,
11+
get_resource_origins_mapping,
812
resource_initialization_event_generator,
913
resource_initialization_manager,
1014
single_resource_event_generator,
@@ -117,3 +121,167 @@ def test_clean_event_generator_exit():
117121
@dg.op
118122
def fake_op(_):
119123
pass
124+
125+
126+
def test_get_resource_origins_mapping():
127+
@dg.resource
128+
def spark(_):
129+
return "spark"
130+
131+
@dg.resource(required_resource_keys={"spark"})
132+
def pyspark(_):
133+
return "pyspark"
134+
135+
@dg.resource
136+
def required_by_nobody(_):
137+
return "nobody"
138+
139+
@dg.op(required_resource_keys={"pyspark"})
140+
def uses_pyspark(_):
141+
pass
142+
143+
@dg.op(required_resource_keys={"spark"})
144+
def uses_spark_directly(_):
145+
pass
146+
147+
@dg.graph
148+
def nested():
149+
uses_spark_directly()
150+
151+
@dg.job(
152+
resource_defs={
153+
"spark": spark,
154+
"pyspark": pyspark,
155+
"required_by_nobody": required_by_nobody,
156+
}
157+
)
158+
def origins_job():
159+
uses_pyspark()
160+
nested()
161+
162+
mapping = get_resource_origins_mapping(origins_job)
163+
164+
assert mapping["pyspark"] == {"uses_pyspark"}
165+
166+
# spark is required transitively (via pyspark) and directly by a nested op, reported by handle
167+
assert mapping["spark"] == {"uses_pyspark", "nested.uses_spark_directly"}
168+
169+
# resources no op requires are absent from the mapping
170+
assert "required_by_nobody" not in mapping
171+
assert "io_manager" not in mapping
172+
173+
174+
def gen_two_op_resource_job():
175+
"""A job where each op requires a different resource."""
176+
177+
@dg.resource
178+
def resource_a(_):
179+
return "a"
180+
181+
@dg.resource
182+
def resource_b(_):
183+
return "b"
184+
185+
@dg.op(required_resource_keys={"resource_a"})
186+
def op_one(_):
187+
pass
188+
189+
@dg.op(required_resource_keys={"resource_b"})
190+
def op_two(_):
191+
pass
192+
193+
@dg.job(resource_defs={"resource_a": resource_a, "resource_b": resource_b})
194+
def two_op_resource_job():
195+
op_one()
196+
op_two()
197+
198+
return two_op_resource_job
199+
200+
201+
def _resource_init_started_message(job_def, *, pass_job_def: bool) -> str:
202+
"""Drives the real resource-init generator and returns the RESOURCE_INIT_STARTED message."""
203+
instance = DagsterInstance.ephemeral()
204+
execution_plan = create_execution_plan(job_def)
205+
run = instance.create_run_for_job(job_def=job_def, execution_plan=execution_plan)
206+
log_manager = DagsterLogManager.create(loggers=[], dagster_run=run)
207+
resolved_run_config = ResolvedRunConfig.build(job_def)
208+
209+
events = list(
210+
resource_initialization_event_generator(
211+
resource_defs=job_def.resource_defs,
212+
resource_configs=resolved_run_config.resources,
213+
log_manager=log_manager,
214+
execution_plan=execution_plan,
215+
dagster_run=run,
216+
resource_keys_to_init=get_required_resource_keys_to_init(execution_plan, job_def),
217+
instance=instance,
218+
emit_persistent_events=True,
219+
event_loop=None,
220+
job_def=job_def if pass_job_def else None,
221+
)
222+
)
223+
224+
started = [
225+
event
226+
for event in events
227+
if isinstance(event, DagsterEvent)
228+
and event.event_type_value == DagsterEventType.RESOURCE_INIT_STARTED.value
229+
]
230+
assert len(started) == 1, f"expected exactly one RESOURCE_INIT_STARTED, got {len(started)}"
231+
return started[0].message
232+
233+
234+
def test_resource_init_message_reports_op_origins():
235+
"""With job_def available, the message says which ops require each resource (issue #2307)."""
236+
message = _resource_init_started_message(gen_two_op_resource_job(), pass_job_def=True)
237+
238+
assert message == (
239+
"Starting initialization of resources:\n"
240+
"io_manager - required by the I/O layer\n"
241+
"resource_a - required by op instances [op_one]\n"
242+
"resource_b - required by op instances [op_two]"
243+
)
244+
245+
246+
def test_resource_init_message_falls_back_without_job_def():
247+
"""Without job_def (e.g. build_resources), the message is byte-for-byte the original."""
248+
message = _resource_init_started_message(gen_two_op_resource_job(), pass_job_def=False)
249+
250+
assert message == "Starting initialization of resources [io_manager, resource_a, resource_b]."
251+
252+
253+
def test_job_def_reaches_innermost_resource_generator(monkeypatch):
254+
"""job_def must be forwarded from resource_initialization_manager down to the innermost
255+
generator. A param that is accepted but not forwarded still typechecks, so assert it arrives.
256+
"""
257+
job_def = gen_two_op_resource_job()
258+
instance = DagsterInstance.ephemeral()
259+
execution_plan = create_execution_plan(job_def)
260+
run = instance.create_run_for_job(job_def=job_def, execution_plan=execution_plan)
261+
log_manager = DagsterLogManager.create(loggers=[], dagster_run=run)
262+
resolved_run_config = ResolvedRunConfig.build(job_def)
263+
264+
captured = {}
265+
real_core_generator = resources_init._core_resource_initialization_event_generator # noqa: SLF001
266+
267+
def spy(**kwargs):
268+
captured["job_def"] = kwargs.get("job_def", "<<not forwarded>>")
269+
return real_core_generator(**kwargs)
270+
271+
monkeypatch.setattr(resources_init, "_core_resource_initialization_event_generator", spy)
272+
273+
manager = resource_initialization_manager(
274+
resource_defs=job_def.resource_defs,
275+
resource_configs=resolved_run_config.resources,
276+
log_manager=log_manager,
277+
execution_plan=execution_plan,
278+
dagster_run=run,
279+
resource_keys_to_init=get_required_resource_keys_to_init(execution_plan, job_def),
280+
instance=instance,
281+
emit_persistent_events=True,
282+
event_loop=None,
283+
job_def=job_def,
284+
)
285+
list(manager.generate_setup_events())
286+
287+
assert captured["job_def"] is job_def

python_modules/libraries/dagstermill/dagstermill/manager.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ def _setup_resources(
8686
instance: DagsterInstance | None,
8787
emit_persistent_events: bool | None,
8888
event_loop: AbstractEventLoop | None,
89+
job_def: JobDefinition | None = None,
8990
):
9091
"""Drop-in replacement for
9192
`dagster._core.execution.resources_init.resource_initialization_manager`. It uses a
@@ -101,6 +102,7 @@ def _setup_resources(
101102
instance=instance,
102103
emit_persistent_events=emit_persistent_events,
103104
event_loop=event_loop,
105+
job_def=job_def,
104106
)
105107
self.resource_manager = DagstermillResourceEventGenerationManager(
106108
generator, ScopedResourcesBuilder

0 commit comments

Comments
 (0)