phet commented on code in PR #3894:
URL: https://github.com/apache/gobblin/pull/3894#discussion_r1522046894


##########
gobblin-temporal/src/main/java/org/apache/gobblin/temporal/workflows/metrics/EventSubmitterContext.java:
##########
@@ -119,6 +70,70 @@ private static Class resolveClass(String 
canonicalClassName) {
     } catch (ClassNotFoundException e) {
       throw new RuntimeException(e);
     }
+  }
+
+  public static class Builder {
+    private List<Tag<?>> tags = new ArrayList<>();
+    private String namespace;
+    public Builder addTag(Tag<?> tag) {
+      this.tags.add(tag);
+      return this;
+    }
 
+    public Builder addTags(List<Tag<?>> tags) {
+      this.tags.addAll(tags);
+      return this;
+    }
+
+    public Builder setNamespace(String namespace) {
+      this.namespace = namespace;
+      return this;
+    }
+
+    public Builder withGaaSJobProps(Properties jobProps) {
+      // TODO: Add temporal specific metadata tags
+
+      if (jobProps.containsKey(ConfigurationKeys.FLOW_GROUP_KEY)) {
+        this.tags.add(new 
Tag<>(TimingEvent.FlowEventConstants.FLOW_GROUP_FIELD, 
jobProps.getProperty(ConfigurationKeys.FLOW_GROUP_KEY)));
+        this.tags.add(new 
Tag<>(TimingEvent.FlowEventConstants.FLOW_NAME_FIELD, 
jobProps.getProperty(ConfigurationKeys.FLOW_NAME_KEY)));
+        this.tags.add(new 
Tag<>(TimingEvent.FlowEventConstants.FLOW_EXECUTION_ID_FIELD, 
jobProps.getProperty(ConfigurationKeys.FLOW_EXECUTION_ID_KEY)));
+      }
+
+      if (jobProps.containsKey(ConfigurationKeys.JOB_CURRENT_ATTEMPTS)) {
+        this.tags.add(new 
Tag<>(TimingEvent.FlowEventConstants.CURRENT_ATTEMPTS_FIELD,
+            jobProps.getProperty(ConfigurationKeys.JOB_CURRENT_ATTEMPTS, 
"1")));
+        this.tags.add(new 
Tag<>(TimingEvent.FlowEventConstants.CURRENT_GENERATION_FIELD,
+            jobProps.getProperty(ConfigurationKeys.JOB_CURRENT_GENERATION, 
"1")));
+        this.tags.add(new 
Tag<>(TimingEvent.FlowEventConstants.SHOULD_RETRY_FIELD,
+            "false"));
+      }
+
+      //Use azkaban.flow.execid as the jobExecutionId
+      this.tags.add(new 
Tag<>(TimingEvent.FlowEventConstants.JOB_EXECUTION_ID_FIELD, "0"));
+
+      this.tags.add(new Tag<>(TimingEvent.FlowEventConstants.JOB_GROUP_FIELD,
+          jobProps.getProperty(ConfigurationKeys.JOB_GROUP_KEY, "")));
+      this.tags.add(new Tag<>(TimingEvent.FlowEventConstants.JOB_NAME_FIELD,
+          jobProps.getProperty(ConfigurationKeys.JOB_NAME_KEY, "")));
+      this.tags.add(new Tag<>(TimingEvent.METADATA_MESSAGE, ""));
+
+      this.tags.add(new Tag<>(Help.USER_TO_PROXY_KEY, 
jobProps.getProperty(Help.USER_TO_PROXY_KEY, "")));
+      return this;
+    }
+
+    public Builder withEventSubmitter(EventSubmitter eventSubmitter) {
+      this.tags.addAll(eventSubmitter.getTags());
+      this.namespace = eventSubmitter.getNamespace();
+      return this;

Review Comment:
   up to you, but personally I'd make this an alt. ctor, since I prefer:
   ```
   new EventSubmitterContext.Builder(eventSubmitter)
       .build();
   ```
   to
   ```
   new EventSubmitterContext.Builder()
       .withEventSubmitter(eventSubmitter)
       .build();
   ```



##########
gobblin-temporal/src/main/java/org/apache/gobblin/temporal/workflows/metrics/EventSubmitterContext.java:
##########
@@ -119,6 +70,70 @@ private static Class resolveClass(String 
canonicalClassName) {
     } catch (ClassNotFoundException e) {
       throw new RuntimeException(e);
     }
+  }
+
+  public static class Builder {
+    private List<Tag<?>> tags = new ArrayList<>();
+    private String namespace;

Review Comment:
   up to you, but personally I would simplify point of use w/ 
`@AllArgsConstructor`



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to