Skip to content

Commit

Permalink
fix: Core sets parent_deployment to a wrong value #530 (#604)
Browse files Browse the repository at this point in the history
  • Loading branch information
astsiapanay authored Dec 10, 2024
1 parent 88e3a73 commit 88437f4
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.google.common.annotations.VisibleForTesting;
import io.netty.buffer.ByteBufInputStream;
import io.vertx.core.Vertx;
import io.vertx.core.buffer.Buffer;
Expand Down Expand Up @@ -105,13 +106,10 @@ private void append(ProxyContext context, LogEntry entry) throws JsonProcessingE
append(entry, context.getDeployment().getName(), true);
append(entry, "\"", false);

String sourceDeployment = context.getSourceDeployment();
if (sourceDeployment != null) {
String initialDeployment = context.getInitialDeployment();
String parentDeployment = getParentDeployment(context);
if (parentDeployment != null) {
append(entry, ",\"parent_deployment\":\"", false);
// if deployment(callee) is configured with interceptors the return initial deployment(which triggers interceptors)
// otherwise return source deployment(caller)
append(entry, initialDeployment != null ? initialDeployment : sourceDeployment, true);
append(entry, parentDeployment, true);
append(entry, "\"", false);
}

Expand Down Expand Up @@ -341,4 +339,24 @@ static boolean isStreamingResponse(@Nullable Buffer response) {
}
return j == dataToken.length();
}

@VisibleForTesting
static String getParentDeployment(ProxyContext context) {
List<String> interceptors = context.getInterceptors();
if (interceptors == null) {
return context.getSourceDeployment();
}
// skip interceptors and return the deployment which called the current one
List<String> executionPath = context.getExecutionPath();
int i = executionPath.size() - 2;
for (int j = interceptors.size() - 1; i >= 0 && j >= 0; i--, j--) {
String deployment = executionPath.get(i);
String interceptor = interceptors.get(j);
if (!deployment.equals(interceptor)) {
log.warn("Can't find parent deployment because interceptor path doesn't match: expected - {}, actual - {}", interceptor, deployment);
return null;
}
}
return i < 0 ? null : executionPath.get(i);
}
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
package com.epam.aidial.core.server.log;

import com.epam.aidial.core.server.ProxyContext;
import io.vertx.core.buffer.Buffer;
import org.junit.jupiter.api.Test;

import java.util.List;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

@SuppressWarnings("checkstyle:LineLength")
public class GfLogStoreTest {
Expand Down Expand Up @@ -192,4 +198,58 @@ public void testAssembleStreamingResponse2() {


}

@Test
public void testGetParentDeployment_NoInterceptors() {
ProxyContext context = mock(ProxyContext.class);
// app calls model without interceptors
when(context.getInterceptors()).thenReturn(null);
when(context.getSourceDeployment()).thenReturn("app");

String result = GfLogStore.getParentDeployment(context);

assertEquals("app", result);
}

@Test
public void testGetParentDeployment_DeploymentWithInterceptors1() {
ProxyContext context = mock(ProxyContext.class);
// app calls model with interceptors
List<String> interceptors = List.of("interceptor1", "interceptor2");
when(context.getInterceptors()).thenReturn(interceptors);
List<String> executionPath = List.of("app", "interceptor1", "interceptor2", "model");
when(context.getExecutionPath()).thenReturn(executionPath);

String result = GfLogStore.getParentDeployment(context);

assertEquals("app", result);
}

@Test
public void testGetParentDeployment_DeploymentWithInterceptors2() {
ProxyContext context = mock(ProxyContext.class);
// chat calls model with interceptors
List<String> interceptors = List.of("interceptor1", "interceptor2");
when(context.getInterceptors()).thenReturn(interceptors);
List<String> executionPath = List.of("interceptor1", "interceptor2", "model");
when(context.getExecutionPath()).thenReturn(executionPath);

String result = GfLogStore.getParentDeployment(context);

assertNull(result);
}

@Test
public void testGetParentDeployment_InterceptorPathMismatch() {
ProxyContext context = mock(ProxyContext.class);
// app calls model with interceptors but interceptor1 calls some dep1 in the middle using the same per request key
List<String> interceptors = List.of("interceptor1", "interceptor2");
when(context.getInterceptors()).thenReturn(interceptors);
List<String> executionPath = List.of("app", "interceptor1", "dep1", "interceptor2", "model");
when(context.getExecutionPath()).thenReturn(executionPath);

String result = GfLogStore.getParentDeployment(context);

assertNull(result);
}
}

0 comments on commit 88437f4

Please sign in to comment.