Skip to content

Commit

Permalink
Include more info in DRS getAccessURL Bard logs
Browse files Browse the repository at this point in the history
Add the snapshot name and billing profile id to the
DRS log for getAccessURL. This will allow us to know
which snapshots and billing profiles (sometimes associated
with a specific project) users are requesting data from.
  • Loading branch information
samanehsan committed Jul 31, 2024
1 parent 97d4cac commit 93f996f
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ private BardEventProperties() {}
public static final String BILLING_PROFILE_ID_FIELD_NAME = "billingProfileId";
public static final String METHOD_FIELD_NAME = "method";
public static final String PATH_FIELD_NAME = "path";
public static final String SNAPSHOT_NAME_FIELD_NAME = "snapshotName";
}
18 changes: 17 additions & 1 deletion src/main/java/bio/terra/service/filedata/DrsService.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import bio.terra.app.controller.exception.TooManyRequestsException;
import bio.terra.app.logging.PerformanceLogger;
import bio.terra.app.model.GoogleRegion;
import bio.terra.app.usermetrics.BardEventProperties;
import bio.terra.app.usermetrics.UserLoggingMetrics;
import bio.terra.common.CloudPlatformWrapper;
import bio.terra.common.FutureUtils;
import bio.terra.common.exception.FeatureNotImplementedException;
Expand Down Expand Up @@ -111,6 +113,7 @@ public class DrsService {
private final DrsDao drsDao;
private final DrsMetricsService drsMetricsService;
private final AsyncTaskExecutor executor;
private final UserLoggingMetrics loggingMetrics;

private final Map<UUID, SnapshotProject> snapshotProjectsCache =
Collections.synchronizedMap(new PassiveExpiringMap<>(15, TimeUnit.MINUTES));
Expand All @@ -133,7 +136,8 @@ public DrsService(
EcmConfiguration ecmConfiguration,
DrsDao drsDao,
DrsMetricsService drsMetricsService,
@Qualifier("drsResolutionThreadpool") AsyncTaskExecutor executor) {
@Qualifier("drsResolutionThreadpool") AsyncTaskExecutor executor,
UserLoggingMetrics loggingMetrics) {
this.snapshotService = snapshotService;
this.fileService = fileService;
this.drsIdService = drsIdService;
Expand All @@ -148,6 +152,7 @@ public DrsService(
this.drsDao = drsDao;
this.drsMetricsService = drsMetricsService;
this.executor = executor;
this.loggingMetrics = loggingMetrics;
}

private class DrsRequestResource implements AutoCloseable {
Expand Down Expand Up @@ -486,6 +491,11 @@ private DRSAccessURL getAccessURL(

BillingProfileModel billingProfileModel = cachedSnapshot.datasetBillingProfileModel;

loggingMetrics.set(BardEventProperties.SNAPSHOT_NAME_FIELD_NAME, cachedSnapshot.getName());
loggingMetrics.set(
BardEventProperties.BILLING_PROFILE_ID_FIELD_NAME,
cachedSnapshot.getSnapshotBillingProfileId());

assertAccessMethodMatchingAccessId(accessId, drsObject);

FSFile fsFile;
Expand Down Expand Up @@ -985,6 +995,7 @@ static <R> List<R> extractDistinctListOfDrsObjectValues(
@VisibleForTesting
static class SnapshotCacheResult {
private final UUID id;
private final String name;
private final boolean isSelfHosted;
private final BillingProfileModel datasetBillingProfileModel;
private final UUID snapshotBillingProfileId;
Expand All @@ -995,6 +1006,7 @@ static class SnapshotCacheResult {

public SnapshotCacheResult(Snapshot snapshot) {
this.id = snapshot.getId();
this.name = snapshot.getName();
this.isSelfHosted = snapshot.isSelfHosted();
this.globalFileIds = snapshot.hasGlobalFileIds();
this.datasetBillingProfileModel =
Expand All @@ -1019,6 +1031,10 @@ public UUID getId() {
return this.id;
}

public String getName() {
return this.name;
}

public UUID getSnapshotBillingProfileId() {
return snapshotBillingProfileId;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,11 @@ void testSendEvent() throws Exception {
}

@Test
void testSendEventWithBillingProfileId() throws Exception {
void testSendEventWithAdditionalProperties() throws Exception {
String snapshotName = "snapshotName";
String billingProfileId = UUID.randomUUID().toString();
eventProperties.set(BardEventProperties.BILLING_PROFILE_ID_FIELD_NAME, billingProfileId);
eventProperties.set(BardEventProperties.SNAPSHOT_NAME_FIELD_NAME, snapshotName);

mockRequestAuth(request);

Expand All @@ -142,7 +144,8 @@ void testSendEventWithBillingProfileId() throws Exception {
Map.of(
BardEventProperties.METHOD_FIELD_NAME, METHOD.toUpperCase(),
BardEventProperties.PATH_FIELD_NAME, REQUEST_URI,
BardEventProperties.BILLING_PROFILE_ID_FIELD_NAME, billingProfileId),
BardEventProperties.BILLING_PROFILE_ID_FIELD_NAME, billingProfileId,
BardEventProperties.SNAPSHOT_NAME_FIELD_NAME, snapshotName),
APP_ID,
DNS_NAME)));

Expand Down
5 changes: 4 additions & 1 deletion src/test/java/bio/terra/service/filedata/DrsServiceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import bio.terra.app.model.AzureRegion;
import bio.terra.app.model.CloudRegion;
import bio.terra.app.model.GoogleRegion;
import bio.terra.app.usermetrics.UserLoggingMetrics;
import bio.terra.common.TestUtils;
import bio.terra.common.UriUtils;
import bio.terra.common.category.Unit;
Expand Down Expand Up @@ -137,6 +138,7 @@ class DrsServiceTest {
@Mock private DrsDao drsDao;
@Mock private ApplicationConfiguration appConfig;
@Mock private DrsMetricsService drsMetricsService;
@Mock private UserLoggingMetrics loggingMetrics;

private DrsIdService drsIdService;
private DrsService drsService;
Expand Down Expand Up @@ -176,7 +178,8 @@ void before() throws Exception {
ecmConfiguration,
drsDao,
drsMetricsService,
new SimpleAsyncTaskExecutor()));
new SimpleAsyncTaskExecutor(),
loggingMetrics));
when(jobService.getActivePodCount()).thenReturn(1);
when(drsConfiguration.maxDrsLookups()).thenReturn(1);

Expand Down

0 comments on commit 93f996f

Please sign in to comment.