[Core] Add task_name, task_function_name and actor_name in Structured Logging #48703

MengjinYan · 2024-11-12T18:11:31Z

Why are these changes needed?

This PR added the task_name, task_function_name and actor_name to the structured logging.

To obtain the task_name and task_function, corresponding run_context methods & tests are added as well.

Related issue number

Close https://github.com/anyscale/rayturbo/issues/857

Checks

I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
I've run scripts/format.sh to lint the changes in this PR.
I've included any doc changes needed for https://docs.ray.io/en/master/.
- I've added any new APIs to the API Reference. For example, if I added a
  method in Tune, I've added it in doc/source/tune/api/ under the
  corresponding .rst file.
I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
Testing Strategy
- Unit tests
- Release tests
- This PR is not tested :(

Signed-off-by: Mengjin Yan <[email protected]>

alanwguo

thanks! lgtm but I'm not the best person to review this logic.

@kevin85421 should review I think.

kevin85421

Overall LGTM. Left some nit comments and questions.

python/ray/_raylet.pyx

kevin85421 · 2024-11-18T22:51:21Z

python/ray/runtime_context.py

+
+        Example:
+
+            .. testcode::


add an example for the async case?

kevin85421 · 2024-11-18T22:51:31Z

python/ray/runtime_context.py

+
+        Example:
+
+            .. testcode::


add an example for the async case?

python/ray/runtime_context.py

Co-authored-by: Kai-Hsun Chen <[email protected]> Signed-off-by: Mengjin Yan <[email protected]>

Signed-off-by: Mengjin Yan <[email protected]>

jjyao · 2024-11-21T01:35:38Z

python/ray/_private/ray_logging/constants.py

@@ -41,6 +41,9 @@ class LogKey(str, Enum):
    NODE_ID = "node_id"
    ACTOR_ID = "actor_id"
    TASK_ID = "task_id"
+    ACTOR_NAME = "actor_name"
+    TASK_NAME = "task_name"
+    TASK_FUNCTION = "task_function"


Do we need task_function? I thought we only need task_name which should be function name by default?

You are right. By default, in general, we the task name as the function name.

I still see 2 differences between task_name & task_function. That's why I kept them both.

It is possible to set a customized task name by specifying name in options, but task function will always be the function call name

The specific string in task_function will contain the python module name comparing to default task_name will only contain the function itself.

@alanwguo do we need function name or task name is enough for now. I feel having task_name is enough.

I've seen task name can explode in cardinality but function name is a bit more stable. Having both could be useful where we can decide which to use depending on performance characteristics. For example if task_name is too high cardinality, we can choose to use func name exclusively.

jjyao

Sorry for the late review. LG, just some naming comments.

jjyao · 2024-11-25T06:26:15Z

python/ray/_private/ray_logging/constants.py

@@ -41,6 +41,9 @@ class LogKey(str, Enum):
    NODE_ID = "node_id"
    ACTOR_ID = "actor_id"
    TASK_ID = "task_id"
+    ACTOR_NAME = "actor_name"
+    TASK_NAME = "task_name"
+    TASK_FUNCTION = "task_function"


nit: task_function_name. If you think it's too long, we can also do task_func_name

Synced offline. Changing to function name will implicitly indicate the parameter type and will be helpful to understand the field.

jjyao · 2024-11-25T06:27:39Z

python/ray/_private/ray_logging/filters.py

+            task_name = runtime_context.get_task_name()
+            if task_name is not None:
+                setattr(record, LogKey.TASK_NAME.value, task_name)
+            task_function = runtime_context.get_task_function()


get_task_function_name

get_task_function makes me feel it might return the FunctionDescriptor

jjyao · 2024-11-25T06:28:22Z

python/ray/_raylet.pyx

@@ -261,6 +261,8 @@ cdef optional[ObjectIDIndexType] NULL_PUT_INDEX = nullopt
 # https://docs.python.org/3/library/contextvars.html#contextvars.ContextVar
 # It is thread-safe.
 async_task_id = contextvars.ContextVar('async_task_id', default=None)
+async_task_name = contextvars.ContextVar('async_task_name', default=None)
+async_task_function = contextvars.ContextVar('async_task_function', default=None)


task_function_name

Signed-off-by: Mengjin Yan <[email protected]>

python/ray/includes/libcoreworker.pxd

Co-authored-by: Jiajun Yao <[email protected]> Signed-off-by: Mengjin Yan <[email protected]>

Signed-off-by: Mengjin Yan <[email protected]>

… Logging (ray-project#48703) Signed-off-by: Mengjin Yan <[email protected]> Signed-off-by: Connor Sanders <[email protected]>

… Logging (ray-project#48703) Signed-off-by: Mengjin Yan <[email protected]> Signed-off-by: hjiang <[email protected]>

MengjinYan added 6 commits November 12, 2024 09:57

add task_name, task_function and actor_id

c522245

Signed-off-by: Mengjin Yan <[email protected]>

fix lint issue

eec0a8e

Signed-off-by: Mengjin Yan <[email protected]>

Merge branch 'master' into issue-857

673c745

fix test failure

3ca9d0e

Signed-off-by: Mengjin Yan <[email protected]>

fix test failure

17f19a3

Signed-off-by: Mengjin Yan <[email protected]>

fix test failure

40b550b

Signed-off-by: Mengjin Yan <[email protected]>

MengjinYan marked this pull request as ready for review November 13, 2024 20:51

Merge branch 'master' into issue-857

bc08318

MengjinYan added the go add ONLY when ready to merge, run all tests label Nov 13, 2024

MengjinYan requested review from kevin85421 and alanwguo November 13, 2024 20:52

MengjinYan added 2 commits November 14, 2024 10:16

Merge branch 'master' into issue-857

2a5200b

fix test failure

e70cbfa

Signed-off-by: Mengjin Yan <[email protected]>

alanwguo reviewed Nov 15, 2024

View reviewed changes

kevin85421 reviewed Nov 18, 2024

View reviewed changes

kevin85421 approved these changes Nov 18, 2024

View reviewed changes

python/ray/runtime_context.py Show resolved Hide resolved

MengjinYan and others added 4 commits November 18, 2024 15:05

Update python/ray/_raylet.pyx

3f7b0a0

Co-authored-by: Kai-Hsun Chen <[email protected]> Signed-off-by: Mengjin Yan <[email protected]>

Update python/ray/_raylet.pyx

ae80ee1

Co-authored-by: Kai-Hsun Chen <[email protected]> Signed-off-by: Mengjin Yan <[email protected]>

fix comment

5bf148b

Signed-off-by: Mengjin Yan <[email protected]>

Merge branch 'master' into issue-857

799a021

jjyao reviewed Nov 21, 2024

View reviewed changes

jjyao changed the title ~~[Core] Add task_name, task_function and actor_id in Structured Logging~~ [Core] Add task_name, task_function and actor_name in Structured Logging Nov 25, 2024

jjyao reviewed Nov 25, 2024

View reviewed changes

MengjinYan added 2 commits November 25, 2024 10:00

fix task function naming

2f484db

Signed-off-by: Mengjin Yan <[email protected]>

Merge branch 'master' into issue-857

64387c7

jjyao changed the title ~~[Core] Add task_name, task_function and actor_name in Structured Logging~~ [Core] Add task_name, task_function_name and actor_name in Structured Logging Nov 25, 2024

jjyao approved these changes Nov 25, 2024

View reviewed changes

python/ray/includes/libcoreworker.pxd Outdated Show resolved Hide resolved

MengjinYan and others added 3 commits November 25, 2024 14:03

Update python/ray/includes/libcoreworker.pxd

eaca9d4

Co-authored-by: Jiajun Yao <[email protected]> Signed-off-by: Mengjin Yan <[email protected]>

Merge branch 'master' into issue-857

36160b7

fix lint issue

506560b

Signed-off-by: Mengjin Yan <[email protected]>

fix lint

7c984cc

Signed-off-by: Mengjin Yan <[email protected]>

jjyao merged commit 58425b9 into master Nov 26, 2024
5 checks passed

jjyao deleted the issue-857 branch November 26, 2024 05:36

dentiny pushed a commit to dentiny/ray that referenced this pull request Dec 7, 2024

[Core] Add task_name, task_function_name and actor_name in Structured…

31402c6

… Logging (ray-project#48703) Signed-off-by: Mengjin Yan <[email protected]> Signed-off-by: hjiang <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Core] Add task_name, task_function_name and actor_name in Structured Logging #48703

[Core] Add task_name, task_function_name and actor_name in Structured Logging #48703

MengjinYan commented Nov 12, 2024 •

edited by jjyao

Loading

alanwguo left a comment

kevin85421 left a comment

kevin85421 Nov 18, 2024

kevin85421 Nov 18, 2024

jjyao Nov 21, 2024

MengjinYan Nov 21, 2024

jjyao Nov 21, 2024

alanwguo Nov 21, 2024

jjyao Nov 21, 2024

jjyao left a comment

jjyao Nov 25, 2024

MengjinYan Nov 25, 2024

jjyao Nov 25, 2024

jjyao Nov 25, 2024

[Core] Add task_name, task_function_name and actor_name in Structured Logging #48703

[Core] Add task_name, task_function_name and actor_name in Structured Logging #48703

Conversation

MengjinYan commented Nov 12, 2024 • edited by jjyao Loading

Why are these changes needed?

Related issue number

Checks

alanwguo left a comment

Choose a reason for hiding this comment

kevin85421 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jjyao left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MengjinYan commented Nov 12, 2024 •

edited by jjyao

Loading