Skip to content

Commit 0ff6555

Browse files
committed
code review comments
1 parent df6f95a commit 0ff6555

File tree

3 files changed

+19
-16
lines changed

3 files changed

+19
-16
lines changed

src/main/java/bio/terra/app/usermetrics/UserLoggingMetrics.java

-5
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,4 @@ public void setAll(HashMap<String, Object> value) {
4646
properties.putAll(value);
4747
metrics.set(properties);
4848
}
49-
50-
@VisibleForTesting
51-
public void reset() {
52-
metrics.get().clear();
53-
}
5449
}

src/main/java/bio/terra/service/filedata/DrsService.java

+18-10
Original file line numberDiff line numberDiff line change
@@ -491,16 +491,7 @@ private DRSAccessURL getAccessURL(
491491

492492
BillingProfileModel billingProfileModel = cachedSnapshot.datasetBillingProfileModel;
493493

494-
loggingMetrics.set(BardEventProperties.DATASET_ID_FIELD_NAME, cachedSnapshot.getDatasetId());
495-
loggingMetrics.set(
496-
BardEventProperties.DATASET_NAME_FIELD_NAME, cachedSnapshot.getDatasetName());
497-
loggingMetrics.set(BardEventProperties.SNAPSHOT_ID_FIELD_NAME, snapshotId);
498-
loggingMetrics.set(BardEventProperties.SNAPSHOT_NAME_FIELD_NAME, cachedSnapshot.getName());
499-
loggingMetrics.set(
500-
BardEventProperties.BILLING_PROFILE_ID_FIELD_NAME,
501-
cachedSnapshot.getSnapshotBillingProfileId());
502-
loggingMetrics.set(
503-
BardEventProperties.CLOUD_PLATFORM_FIELD_NAME, cachedSnapshot.getCloudPlatform());
494+
logAdditionalProperties(cachedSnapshot);
504495

505496
assertAccessMethodMatchingAccessId(accessId, drsObject);
506497

@@ -526,6 +517,23 @@ private DRSAccessURL getAccessURL(
526517
}
527518
}
528519

520+
/**
521+
* Include additional properties in the event log sent to Bard
522+
* @param cachedSnapshot
523+
*/
524+
private void logAdditionalProperties(SnapshotCacheResult cachedSnapshot) {
525+
loggingMetrics.set(BardEventProperties.DATASET_ID_FIELD_NAME, cachedSnapshot.getDatasetId());
526+
loggingMetrics.set(
527+
BardEventProperties.DATASET_NAME_FIELD_NAME, cachedSnapshot.getDatasetName());
528+
loggingMetrics.set(BardEventProperties.SNAPSHOT_ID_FIELD_NAME, cachedSnapshot.getId());
529+
loggingMetrics.set(BardEventProperties.SNAPSHOT_NAME_FIELD_NAME, cachedSnapshot.getName());
530+
loggingMetrics.set(
531+
BardEventProperties.BILLING_PROFILE_ID_FIELD_NAME,
532+
cachedSnapshot.getSnapshotBillingProfileId());
533+
loggingMetrics.set(
534+
BardEventProperties.CLOUD_PLATFORM_FIELD_NAME, cachedSnapshot.getCloudPlatform());
535+
}
536+
529537
@VisibleForTesting
530538
DrsId resolveDrsObjectId(String drsObjectId) {
531539
// If the drsObjectId is not a TDR generated DRS ID, then assume it's an alias and resolve it

src/test/java/bio/terra/app/usermetrics/UserMetricsInterceptorTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ class UserMetricsInterceptorTest {
7070

7171
@BeforeEach
7272
void setUp() {
73-
eventProperties.reset();
73+
eventProperties.get().clear();
7474
when(metricsConfig.ignorePaths()).thenReturn(List.of());
7575
when(metricsConfig.appId()).thenReturn(APP_ID);
7676
when(metricsConfig.bardBasePath()).thenReturn(BARD_BASE_PATH);

0 commit comments

Comments
 (0)