Add pod name to TaskLocation for easier observability and debugging.#14758
Add pod name to TaskLocation for easier observability and debugging.#14758suneet-s merged 7 commits intoapache:masterfrom
Conversation
| private final int tlsPort; | ||
|
|
||
| @Nullable | ||
| private final String k8sPodName; |
There was a problem hiding this comment.
Should we use a more generic field name here instead of named it after k8s? Maybe just name it jobName
There was a problem hiding this comment.
hmm, is there another use case for using this field? it doesn't show up in the console if its not set so i figured its okay to leave it as more specific
There was a problem hiding this comment.
I think k8sJobName will be a good name here as this is what it is called in the code in K8sTaskId
...overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/KubernetesPeonLifecycle.java
Outdated
Show resolved
Hide resolved
| private final int tlsPort; | ||
|
|
||
| @Nullable | ||
| private final String k8sPodName; |
There was a problem hiding this comment.
I think k8sJobName will be a good name here as this is what it is called in the code in K8sTaskId
| Assert.assertNull(noK8sJobName.getK8sPodName()); | ||
|
|
||
| TaskLocation k8sJobName = TaskLocation.create("foo", 1, 2, false, "job-name"); | ||
| Assert.assertEquals("job-name", k8sJobName.getK8sPodName()); |
There was a problem hiding this comment.
nit: it would be better to split this test into 2
There was a problem hiding this comment.
k8sJobName != k8sPodName, in this case i think the pod name is more useful but we could add both if needed?
…a/org/apache/druid/k8s/overlord/KubernetesPeonLifecycle.java Co-authored-by: Suneet Saldanha <suneet@apache.org>
…nto addPodNameToLocation
…14758) * Add pod name to location * Add log * fix style * Update extensions-contrib/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/KubernetesPeonLifecycle.java Co-authored-by: Suneet Saldanha <suneet@apache.org> * Fix unit tests --------- Co-authored-by: Suneet Saldanha <suneet@apache.org>
…pache#14758) * Add pod name to location * Add log * fix style * Update extensions-contrib/kubernetes-overlord-extensions/src/main/java/org/apache/druid/k8s/overlord/KubernetesPeonLifecycle.java Co-authored-by: Suneet Saldanha <suneet@apache.org> * Fix unit tests --------- Co-authored-by: Suneet Saldanha <suneet@apache.org>
4 participants
Adds pod name to the TaskLocation object for Kubernetes task scheduling to make debugging easier.
Description
Since the task id <-> kubernetse pod name mapping is not direct and hard to figure out, its helpful for debugging to show the pod name in the druid console. I think it makes sense to add this to task location since the pod the task is running on is sort of like the location when running tasks in K8s.
I also added a log message on the overlord when the task location is set because this is useful for forensics (taskLocation is cleared when the task finishes)
Fixed the bug ...
Renamed the class ...
Added a forbidden-apis entry ...
I thought about just putting the pod name into the task object itself, but i liked the idea of having it in taskLocation in case we ever have some concept of retrying tasks on pods (which would result in a task running as a different pod name)
Release note
Key changed/added classes in this PR
This is what the task location looks like in the druid console now.
If the field is null (e.g. when not running the k8s task runner) nothing will show up under k8sPodName.
This PR has: