[GitHub] [flink-kubernetes-operator] gyfora commented on a diff in pull request #620: [FLINK-32317] Enrich metadata in CR error field

["gyfora (via GitHub)" <gi...@apache.org>]

gyfora commented on code in PR #620:
URL: https://github.com/apache/flink-kubernetes-operator/pull/620#discussion_r1258231715


##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/utils/FlinkResourceExceptionUtils.java:
##########
@@ -129,26 +144,56 @@ public static Optional<String> getSubstringWithMaxLength(String str, int limit)
     private static void enrichMetadata(
             Throwable throwable,
             FlinkResourceException flinkResourceException,
-            int lengthThreshold) {
+            int lengthThreshold,
+            Map<String, String> labelMapper) {
+        if (flinkResourceException.getAdditionalMetadata() == null) {
+            flinkResourceException.setAdditionalMetadata(new HashMap<>());
+        }
+
         if (throwable instanceof RestClientException) {
-            flinkResourceException.setAdditionalMetadata(
-                    Map.of(
+            flinkResourceException
+                    .getAdditionalMetadata()
+                    .put(
                             "httpResponseCode",
-                            ((RestClientException) throwable).getHttpResponseStatus().code()));
+                            ((RestClientException) throwable).getHttpResponseStatus().code());
         }
 
         if (throwable instanceof DeploymentFailedException) {
             getSubstringWithMaxLength(
                             ((DeploymentFailedException) throwable).getReason(), lengthThreshold)
                     .ifPresent(
                             reason ->
-                                    flinkResourceException.setAdditionalMetadata(
-                                            Map.of("reason", reason)));
+                                    flinkResourceException
+                                            .getAdditionalMetadata()
+                                            .put("reason", reason));
         }
 
+        labelMapper
+                .entrySet()
+                .forEach(
+                        (entry) -> {
+                            Pattern pattern = Pattern.compile(entry.getKey());
+
+                            org.apache.flink.util.ExceptionUtils.findThrowable(
+                                            throwable, t -> pattern.matcher(t.getMessage()).find())

Review Comment:
   Can you please add a test for Exceptions with null message? (such as a nullpointer) I think this code would fail



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org