Optionally capture more frames in computations

[gjoseph92]

Adds a distributed.diagnostics.computations.nframes parameter, controlling how many frames of client code are captured in Computations.

Sets the default value to 0, so by default, no code is captured. I can easily change this to 1 if we'd prefer. But 0 seemed like a reasonable default, since:

1. Capturing the code is pretty quick, but has a non-zero time cost, especially if you're calling submit in a loop.
2. This captured code isn't displayed to users anywhere right now. I doubt anyone besides Coiled is using this feature?
3. Systems that want to turn on code-capture (like Coiled) can do so by modifying client-side dask config

Also changes Computation.code from holding strings to tuples of strings.

• Tests added / passed
• Passes pre-commit run --all-files

cc @ntabris

[ntabris]

@gjoseph92 is there already a pattern is dask-land for modifying client-side config? I'm not currently aware of a way to override the default but also not override if user sets this to non-default value? Would we say that if you're using Coiled, then distributed.diagnostics.computations.nframes is ignored and you should use (eg) coiled.analytics.computations.nframes instead?

[github-actions]

Unit Test Results

See test report for an extended history of previous test failures. This is useful for diagnosing flaky tests.

       26 files  +       15         26 suites  +15   13h 28m 11s ⏱️ + 9h 20m 12s
  3 546 tests +         2    3 439 ✔️ +       21     106 💤  -      17  1 ❌  - 1 
44 858 runs  +27 714  42 620 ✔️ +26 401  2 237 💤 +1 315  1 ❌  - 1 

For more details on these failures, see this check.

Results for commit 77893e2. ± Comparison against base commit 37ce1a0.

♻️ This comment has been updated with latest results.

[fjetter]

is there already a pattern is dask-land for modifying client-side config?

theoretically, you can set config values in import or when running certain functions using

dask.config.set({"value": 42})

There is no way to register default fallbacks for certain configuration values (yet)

[ntabris]

Magic string here doesn't seem ideal, what about using None when we can't get code?

[gjoseph92]

This is inserted into the performance report; _get_computation_code would have returned "<Code not available>" before this PR, so I wanted to just keep the same behavior.

[ntabris]

For Coiled I think it's important that we're able to set the default to something non-zero, since we want to collect code by default, while also letting user override. Is there a good way to do that? According to the config doc, dask defaults take precedence over project defaults.

[gjoseph92]

Yeah, I think the problem is that we can't distinguish "0 because it's the default" from "0 because the user is explicitly setting it to override Coiled setting it to something non-zero". We could make the default None in config, and convert None to 0 in code? (Not as ideal but easier to override.)

[gjoseph92]

I dislike this, but another option would be for the coiled package to set distributed.diagnostics.computations.nframes at import time. Then if you have a dask.config.set in your code (most likely after the import), it'll be overridden again.

[fjetter]

An issue was raised that reports the slowness of this code collection in #7740 which would motivate disabling this by default.

There is a not-welll-hardly-at-all documented way for third parties to register defaults, IIUC coiled could set it's default here on import time ( and call config.refresh). If this does not work we should take care of this issue in another PR.
It is documented that this feature exists but not how it is done (FWIW the example mentions distributed but distributed is not using this, maybe it should for some things...)

@gjoseph92 can you rebase/merge main please?

[gjoseph92]

@fjetter merged main. Agreed that it seems we should merge this to turn the behavior off by default, then look into how Coiled will set the config in a separate PR.

It looks like distributed is using dask.config.update_defaults:

distributed/distributed/config.py

Lines 21 to 24 in 77893e2

	with open(fn) as f:
	defaults = yaml.safe_load(f)
	dask.config.update_defaults(defaults)

That sounds pretty much like what we're looking for for Coiled. It just automates what you suggested of adding to defaults, then refreshing the global config.

[fjetter]

It looks like distributed is using dask.config.update_defaults:

That's interesting. There's always something new to discover...