Enhanced crash logs events for: report_crash_loop, pod_oom_kill_enricher, image_pull_backoff_reporter, KubePodCrashLooping #1282

ganeshrvel · 2024-02-09T23:11:12Z

report_crash_loop
pod_oom_kill_enricher
image_pull_backoff_reporter
KubePodCrashLooping

- report_crash_loop - pod_oom_kill_enricher - image_pull_backoff_reporter - KubePodCrashLooping

arikalon1

nice work
left some comments

arikalon1 · 2024-02-18T17:01:04Z

src/robusta/core/playbooks/pod_utils/pending_pod_utils.py

+    blocks = [TableBlock(
+        [[k, v] for (k, v) in pending_rows],
+        ["label", "value"],
+        table_name="",


This is redundant
"" is the default

arikalon1 · 2024-02-18T17:01:46Z

src/robusta/core/playbooks/pod_utils/pending_pod_utils.py

+    return Enrichment(
+        enrichment_type=EnrichmentType.crash_info,
+        blocks=blocks,
+        title="Pod Unscheduled Information")


I would change to:
Unscheduled Pod Information

arikalon1 · 2024-02-18T17:02:27Z

src/robusta/core/playbooks/pod_utils/pending_pod_utils.py

-    blocks: List[BaseBlock] = []
-    investigator = PendingInvestigator(pod)
-    all_reasons = investigator.investigate()
+def get_pending_pod_enrichment(pod: Pod) -> Optional[Enrichment]:


Why is this Optional? We never return None

arikalon1 · 2024-02-18T17:05:07Z

src/robusta/core/playbooks/pod_utils/imagepull_utils.py

@@ -163,6 +183,10 @@ class ImagePullBackoffInvestigator:
            "err_template": 'Error response from daemon: manifest for .*? not found: manifest unknown: Failed to fetch ".*?"',
            "reason": ImagePullBackoffReason.ImageDoesntExist | ImagePullBackoffReason.TagNotFound,
        },
+        {
+            "err_template": r'Error response from daemon: Get ".*?": net\/http: request canceled while waiting for connection \(Client\.Timeout exceeded while awaiting headers\)',


Can we make it less sensitive?
Like anything that includes "Timeout exceeded"
What's the downside of that?

arikalon1 · 2024-02-18T17:05:46Z

src/robusta/core/playbooks/pod_utils/pending_pod_utils.py

-    investigator = PendingInvestigator(pod)
-    all_reasons = investigator.investigate()
+def get_pending_pod_enrichment(pod: Pod) -> Optional[Enrichment]:
+    pending_rows: List[List[str]] = []
    message = get_unscheduled_message(pod)


Does this cover all the pending scenarios ?

It did cover the required fields in the new enrichment sheet. But i have reverted back the MarkdownBlock which had the reason for the pending state

arikalon1 · 2024-02-19T07:34:41Z

playbooks/robusta_playbooks/pod_investigator_enricher.py

-        event.add_enrichment(blocks)
+    if pod_issues_enrichments:
+        issue_message, issues_enrichments = pod_issues_enrichments
+        blocks: List[BaseBlock] = [MarkdownBlock(f"{message_string}. {issue_message}")]


This should be added to description, and not as Markdown

arikalon1 · 2024-02-19T07:43:09Z

playbooks/robusta_playbooks/pod_investigator_enricher.py

+    if len(pods_with_issue) < 1:
+        logging.debug(f"`pods_with_issue` for found for issue: {issue}")
+        return
+
    expected_pods = get_expected_replicas(event)


I think this needs to change
We have here the event, the type of the issue, the wait message and wait reason (which are currently not in use)

We need a new function, get_explanation
Information about number of available pods, and number of unavailable should be taken from the resource status

For Deployment, Statefulset, DaemonSet, message should be:
X pods are available, Y pods are not ready due to $ISSUE TYPE.
($WAIT_REASON: $WAIT_MESSAGE)

For others (Pod, Jobs ...)
Pod is in not ready due to $ISSUE_TYPE

arikalon1 · 2024-02-19T07:45:18Z

src/robusta/core/playbooks/crash_reporter.py

+    )
+
+    enrichments = get_crash_report_enrichments(pod)
+    if enrichments:


This is neven None

arikalon1 · 2024-02-19T07:47:57Z

src/robusta/core/playbooks/crash_reporter.py

+                    "is_empty": True,
+                    "remarks": f"Logs unavailable for container: {container_status.name}"
+                }
+                logging.error(


I think this should be changed to info or removed.
Also, no point printing the container log, we know it's empty

replaced it with logging.info

arikalon1 · 2024-02-19T07:49:30Z

src/robusta/core/playbooks/crash_reporter.py

+            )
+            metadata = None
+            if not container_log:
+                metadata = {


Is this for the UI? To present an empty log?
How does this look in Slack and the other Sinks?

Yes this for the platform UI, to display an empty log.

Slack and msteams sinks screenshots:

How come we don't see the empty file on Slack?

On any event, is there a way to send the empty log only to the UI sink?
Maybe we can create a new type of block, EmptyFileBlock that is handled only in the UI sink? (and sends it as an empty file)

Or other solutions that will have a similar result?

optimized the enrichments new pods counting logic

Fixed crash pod log details

arikalon1

nice works

Added few more comments

arikalon1 · 2024-02-24T09:32:23Z

playbooks/robusta_playbooks/event_enrichments.py

@@ -245,8 +245,12 @@ def deployment_events_enricher(event: DeploymentEvent, params: ExtendedEventEnri
                    event.add_enrichment([events_table_block], {SlackAnnotations.ATTACHMENT: True},
                                         enrichment_type=EnrichmentType.k8s_events, title="Deployment Events")
    else:
-        pods = list_pods_using_selector(dep.metadata.namespace, dep.spec.selector, "status.phase=Running")
-        event.add_enrichment([MarkdownBlock(f"*Replicas: Desired ({dep.spec.replicas}) --> Running ({len(pods)})*")])
+        if dep.kind in ["Deployment", "Statefulset", "DaemonSet"]:


This is a DeploymentEvent , kind is always a deployment
In addition, I think this logic is not correct for DaemonSet

arikalon1 · 2024-02-24T09:39:02Z

playbooks/robusta_playbooks/pod_investigator_enricher.py

+    else:
+        issue_text = issue.name
+
+    if resource.kind in ["Deployment", "Statefulset", "DaemonSet"]:


I think we need a small fix here:
For DaemonSet the available and desired status fields are different.
I think you want:
desired = desiredNumberScheduled
available = numberAvailable
When you run kubectl get DaemonSet -o yaml some-ds you see in status:

status: currentNumberScheduled: 4 desiredNumberScheduled: 4 numberAvailable: 4 numberMisscheduled: 0 numberReady: 4 observedGeneration: 1 updatedNumberScheduled: 4

arikalon1 · 2024-02-24T09:42:26Z

playbooks/robusta_playbooks/pod_investigator_enricher.py


+        event.extend_description(message_text)


I think we should always add it to description, and not only if we found an enrichment

arikalon1 · 2024-02-24T10:03:16Z

src/robusta/core/playbooks/crash_reporter.py

+            )
+            metadata = None
+            if not container_log:
+                metadata = {


How come we don't see the empty file on Slack?

On any event, is there a way to send the empty log only to the UI sink?
Maybe we can create a new type of block, EmptyFileBlock that is handled only in the UI sink? (and sends it as an empty file)

Or other solutions that will have a similar result?

arikalon1 · 2024-02-24T10:06:02Z

src/robusta/core/playbooks/oom_killer_utils.py

-            title="Pod Logs"
-        )
+        metadata = None
+        if not log_data:


same here regarding the empty logs

done. Added new EmptyFileBlock

- report_crash_loop - pod_oom_kill_enricher - image_pull_backoff_reporter - KubePodCrashLooping

optimized the enrichments new pods counting logic

Fixed crash pod log details

… into feature/enhance-crashlog-events

Optimized Events handling

arikalon1 · 2024-02-26T19:59:31Z

src/robusta/core/reporting/blocks.py

@@ -130,6 +130,35 @@ def truncate_content(self, max_file_size_bytes: int) -> bytes:
        return "\n".join(truncated_lines).encode("utf-8")


+class EmptyFileBlock(FileBlock):


I think this should not extend a FileBlock
The point of not making it a FileBlock, was to not send it to all existing sinks.

But if this extends a FileBlock, it will be handled as a FileBlock by all sinks, right?

… into feature/enhance-crashlog-events

arikalon1

nice work

Enhanced crash logs events for:

61bd40d

- report_crash_loop - pod_oom_kill_enricher - image_pull_backoff_reporter - KubePodCrashLooping

ganeshrvel changed the title ~~Enhanced crash logs events for:~~ Enhanced crash logs events for: report_crash_loop, pod_oom_kill_enricher, image_pull_backoff_reporter, KubePodCrashLooping Feb 9, 2024

Fixed send_crash_report import error

465ab55

arikalon1 reviewed Feb 19, 2024

View reviewed changes

ganeshrvel added 8 commits February 21, 2024 03:31

Code cleanup.

9e7607a

optimized the enrichments new pods counting logic

Fixed enrichment type

d1b1431

Fixed crash pod log details

Better replica available text and better regex

54dfddc

Better description text

eaea9ea

Better pod text

0d73128

Pending pod markdown reverted.

013d63f

Code cleaning

59a1e50

Removed RSA pair

5e6de4c

arikalon1 reviewed Feb 24, 2024

View reviewed changes

ganeshrvel and others added 13 commits February 26, 2024 13:35

Enhanced crash logs events for:

ea6bc6a

- report_crash_loop - pod_oom_kill_enricher - image_pull_backoff_reporter - KubePodCrashLooping

Fixed send_crash_report import error

7bb909d

Code cleanup.

847be94

optimized the enrichments new pods counting logic

Fixed enrichment type

83f1bd4

Fixed crash pod log details

Better replica available text and better regex

efddd00

Better description text

92955d2

Better pod text

ee56d8a

Pending pod markdown reverted.

caf0702

Code cleaning

5472eb9

Merge remote-tracking branch 'origin/feature/enhance-crashlog-events'…

5e852da

… into feature/enhance-crashlog-events

Merge branch 'master' into feature/enhance-crashlog-events

efb3cea

Optimized the empty file handling

d059757

Optimized Events handling

Merge branch 'master' into feature/enhance-crashlog-events

d236670

arikalon1 reviewed Feb 26, 2024

View reviewed changes

ganeshrvel and others added 3 commits February 27, 2024 12:15

Optimized the empty file block

e541d01

Merge remote-tracking branch 'origin/feature/enhance-crashlog-events'…

7cb3b82

… into feature/enhance-crashlog-events

Merge branch 'master' into feature/enhance-crashlog-events

3c545f4

arikalon1 approved these changes Feb 27, 2024

View reviewed changes

arikalon1 merged commit ca11c28 into master Feb 27, 2024
16 checks passed

arikalon1 deleted the feature/enhance-crashlog-events branch February 27, 2024 08:12

		@@ -130,6 +130,35 @@ def truncate_content(self, max_file_size_bytes: int) -> bytes:
		return "\n".join(truncated_lines).encode("utf-8")


		class EmptyFileBlock(FileBlock):

Enhanced crash logs events for: report_crash_loop, pod_oom_kill_enricher, image_pull_backoff_reporter, KubePodCrashLooping #1282

Enhanced crash logs events for: report_crash_loop, pod_oom_kill_enricher, image_pull_backoff_reporter, KubePodCrashLooping #1282

Conversation

ganeshrvel commented Feb 9, 2024

arikalon1 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

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

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

ganeshrvel Feb 21, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arikalon1 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

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

arikalon1 left a comment

Choose a reason for hiding this comment

ganeshrvel Feb 21, 2024 •

edited