Skip to content

refactor: add ability to restore context #111

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.

Sign up for GitHub

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 1 commit into from
Sep 23, 2022
Merged

refactor: add ability to restore context #111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions ...l-grpc-utils/src/main/java/org/hypertrace/core/graphql/utils/grpc/GrpcContextBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
package org.hypertrace.core.graphql.utils.grpc;

import java.util.Optional;
import org.hypertrace.core.graphql.context.GraphQlRequestContext;
import org.hypertrace.core.grpcutils.context.RequestContext;

public interface GrpcContextBuilder {
RequestContext build(GraphQlRequestContext requestContext);

Optional<GraphQlRequestContext> tryRestore(RequestContext requestContext);
}
12 changes: 12 additions & 0 deletions ...tils/src/main/java/org/hypertrace/core/graphql/utils/grpc/PlatformGrpcContextBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
import static org.hypertrace.core.grpcutils.context.RequestContextConstants.REQUEST_ID_HEADER_KEY;
import static org.hypertrace.core.grpcutils.context.RequestContextConstants.TENANT_ID_HEADER_KEY;

import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
import java.time.Duration;
import java.util.Arrays;
import java.util.Collection;
import java.util.Map;
Expand All @@ -16,8 +19,12 @@

class PlatformGrpcContextBuilder implements GrpcContextBuilder {

private final Cache<String, GraphQlRequestContext> contextCache =
CacheBuilder.newBuilder().expireAfterAccess(Duration.ofMinutes(1)).maximumSize(1000).build();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used a small short lived cache to do this as the actual transformation to grpc context is lossy - it's not possible to recreate the initial context directly, and the period of time we need to be able to restore a context is typically very small (the time it takes to build the request to the time it takes to make the request).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any chance the max size will be hit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it could (this would be > 1000 requests per minute), but that's not actually an issue since guava caches are LRU and it'll just help us evict entries quicker (we only need them for a short order of time, typically on the order of millis). The issue would be if we have to handle more than the cache size (1000) concurrently which is far beyond the current scale. If it makes you feel better I can make it configurable :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. So it's rare a cache item stays in there for more than a couple of milliseconds?

Copy link
Contributor Author

@aaron-steinfeld aaron-steinfeld Sep 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's rare that we need it to, but the only mechanisms that remove it currently are the size and time expirations.

The expected flow is that when making a remote request in a DAO class, an impl will trade their GQL context for a GRPC context. The GRPC context is used to make the request. The need for this class is for any hooks at the GRPC level - if we're instrumenting or intercepting the calls, we can get the GRPC context that was used to make them, but we can't (without this PR) get the GQL context that was used to create that GRPC context. If we want to make other calls into GQL code in those interceptor classes, we need that GQL context back (so the expected elapsed time here is from initiating to intercepting a client request - at least for my use case)


@Override
public RequestContext build(GraphQlRequestContext requestContext) {
this.contextCache.put(requestContext.getRequestId(), requestContext);
Map<String, String> grpcHeaders =
this.mergeMaps(
requestContext.getTracingContextHeaders(),
Expand All @@ -30,6 +37,11 @@ public RequestContext build(GraphQlRequestContext requestContext) {
return this.build(grpcHeaders);
}

@Override
public Optional<GraphQlRequestContext> tryRestore(RequestContext requestContext) {
return requestContext.getRequestId().map(this.contextCache::getIfPresent);
}

private RequestContext build(@Nonnull Map<String, String> headers) {
RequestContext platformContext = new RequestContext();
headers.forEach(platformContext::add);
Expand Down
9 changes: 9 additions & 0 deletions .../src/test/java/org/hypertrace/core/graphql/utils/grpc/PlatformGrpcContextBuilderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import java.util.Map;
import java.util.Optional;
import org.hypertrace.core.graphql.context.GraphQlRequestContext;
import org.hypertrace.core.grpcutils.context.RequestContext;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
Expand Down Expand Up @@ -61,4 +62,12 @@ void passesAuthHeaderToPlatformContextIfPresent() {
"auth header",
this.builder.build(this.mockRequestContext).getRequestHeaders().get("authorization"));
}

@Test
void testRestoreContext() {
when(this.mockRequestContext.getRequestId()).thenReturn("request id");
RequestContext resultContext = this.builder.build(this.mockRequestContext);
assertEquals(Optional.empty(), this.builder.tryRestore(RequestContext.forTenantId("other")));
assertEquals(Optional.of(this.mockRequestContext), this.builder.tryRestore(resultContext));
}
}