Skip to content

[DR-3476] Include additional fields in DRS getAccessURL Bard logs #1753

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 5, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
Loading