gemini-code-assist[bot] commented on code in PR #38785:
URL: https://github.com/apache/beam/pull/38785#discussion_r3348495980
##########
sdks/java/harness/src/main/java/org/apache/beam/fn/harness/FnHarness.java:
##########
@@ -296,6 +296,12 @@ public static void main(
// Register standard file systems.
FileSystems.setDefaultPipelineOptions(options);
CoderTranslation.verifyModelCodersRegistered();
+ SdkHarnessOptions sdkHarnessOptions =
options.as(SdkHarnessOptions.class);
+ Map<String, String> openTelemetryProperties =
sdkHarnessOptions.getOpenTelemetryProperties();
+ if (openTelemetryProperties != null) {
+ openTelemetryProperties.forEach(System::setProperty);
+ LOG.info("Enabled Open Telemetry");
+ }
Review Comment:
If `openTelemetryProperties` is empty, logging "Enabled Open Telemetry" is
misleading. Additionally, `System.setProperty` throws a `NullPointerException`
if either the key or value is `null`. It is safer to check if the map is not
empty and guard against `null` keys or values before setting them.
```suggestion
SdkHarnessOptions sdkHarnessOptions =
options.as(SdkHarnessOptions.class);
Map<String, String> openTelemetryProperties =
sdkHarnessOptions.getOpenTelemetryProperties();
if (openTelemetryProperties != null &&
!openTelemetryProperties.isEmpty()) {
openTelemetryProperties.forEach((k, v) -> {
if (k != null && v != null) {
System.setProperty(k, v);
}
});
LOG.info("Enabled Open Telemetry with properties: {}",
openTelemetryProperties);
}
```
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/StreamingDataflowWorker.java:
##########
@@ -1042,6 +1043,13 @@ public static void main(String[] args) throws Exception {
WindowedValues.FullWindowedValueCoder.setMetadataSupported();
}
+ SdkHarnessOptions sdkHarnessOptions = options.as(SdkHarnessOptions.class);
+ Map<String, String> openTelemetryProperties =
sdkHarnessOptions.getOpenTelemetryProperties();
+ if (openTelemetryProperties != null) {
+ openTelemetryProperties.forEach(System::setProperty);
+ LOG.info("Enabled Open Telemetry");
+ }
Review Comment:
If `openTelemetryProperties` is empty, logging "Enabled Open Telemetry" is
misleading. Additionally, `System.setProperty` throws a `NullPointerException`
if either the key or value is `null`. It is safer to check if the map is not
empty and guard against `null` keys or values before setting them.
```suggestion
SdkHarnessOptions sdkHarnessOptions =
options.as(SdkHarnessOptions.class);
Map<String, String> openTelemetryProperties =
sdkHarnessOptions.getOpenTelemetryProperties();
if (openTelemetryProperties != null &&
!openTelemetryProperties.isEmpty()) {
openTelemetryProperties.forEach((k, v) -> {
if (k != null && v != null) {
System.setProperty(k, v);
}
});
LOG.info("Enabled Open Telemetry with properties: {}",
openTelemetryProperties);
}
```
--
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]
