Skip to content
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

Backport/backport 12005 to 2.12 #12222

Merged
merged 1 commit into from
Feb 7, 2024
Merged

Backport/backport 12005 to 2.12 #12222

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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- [BUG] Fix remote shards balancer when filtering throttled nodes ([#11724](https://github.com/opensearch-project/OpenSearch/pull/11724))
- [Bug] Check phase name before SearchRequestOperationsListener onPhaseStart ([#12094](https://github.com/opensearch-project/OpenSearch/pull/12094))
- Add advance(int) for numeric values in order to allow point based optimization to kick in ([#12089](https://github.com/opensearch-project/OpenSearch/pull/12089))
- Fix Span operation names generated from RestActions ([#12005](https://github.com/opensearch-project/OpenSearch/pull/12005))

### Security

Expand Down
5 changes: 5 additions & 0 deletions server/src/main/java/org/opensearch/telemetry/tracing/AttributeNames.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ private AttributeNames() {
*/
public static final String HTTP_URI = "http.uri";

/**
* Http Request Query Parameters.
*/
public static final String HTTP_REQ_QUERY_PARAMS = "url.query";

/**
* Rest Request ID.
*/
Expand Down
36 changes: 31 additions & 5 deletions server/src/main/java/org/opensearch/telemetry/tracing/SpanBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.opensearch.action.bulk.BulkShardRequest;
import org.opensearch.action.support.replication.ReplicatedWriteRequest;
import org.opensearch.common.annotation.InternalApi;
import org.opensearch.common.collect.Tuple;
import org.opensearch.core.common.Strings;
import org.opensearch.http.HttpRequest;
import org.opensearch.rest.RestRequest;
Expand Down Expand Up @@ -75,7 +76,9 @@ public static SpanCreationContext from(String spanName, String nodeId, Replicate
}

private static String createSpanName(HttpRequest httpRequest) {
return httpRequest.method().name() + SEPARATOR + httpRequest.uri();
Tuple<String, String> uriParts = splitUri(httpRequest.uri());
String path = uriParts.v1();
return httpRequest.method().name() + SEPARATOR + path;
}

private static Attributes buildSpanAttributes(HttpRequest httpRequest) {
Expand All @@ -84,9 +87,26 @@ private static Attributes buildSpanAttributes(HttpRequest httpRequest) {
.addAttribute(AttributeNames.HTTP_METHOD, httpRequest.method().name())
.addAttribute(AttributeNames.HTTP_PROTOCOL_VERSION, httpRequest.protocolVersion().name());
populateHeader(httpRequest, attributes);

Tuple<String, String> uriParts = splitUri(httpRequest.uri());
String query = uriParts.v2();
if (query.isBlank() == false) {
attributes.addAttribute(AttributeNames.HTTP_REQ_QUERY_PARAMS, query);
}

return attributes;
}

private static Tuple<String, String> splitUri(String uri) {
int index = uri.indexOf('?');
if (index >= 0 && index < uri.length() - 1) {
String path = uri.substring(0, index);
String query = uri.substring(index + 1);
return new Tuple<>(path, query);
}
return new Tuple<>(uri, "");
}

private static void populateHeader(HttpRequest httpRequest, Attributes attributes) {
HEADERS_TO_BE_ADDED_AS_ATTRIBUTES.forEach(x -> {
if (httpRequest.getHeaders() != null
Expand All @@ -102,9 +122,8 @@ private static String createSpanName(RestRequest restRequest) {
if (restRequest != null) {
try {
String methodName = restRequest.method().name();
// path() does the decoding, which may give error
String path = restRequest.path();
spanName = methodName + SEPARATOR + path;
String rawPath = restRequest.rawPath();
spanName = methodName + SEPARATOR + rawPath;
} catch (Exception e) {
// swallow the exception and keep the default name.
}
Expand All @@ -114,9 +133,16 @@ private static String createSpanName(RestRequest restRequest) {

private static Attributes buildSpanAttributes(RestRequest restRequest) {
if (restRequest != null) {
return Attributes.create()
Attributes attributes = Attributes.create()
.addAttribute(AttributeNames.REST_REQ_ID, restRequest.getRequestId())
.addAttribute(AttributeNames.REST_REQ_RAW_PATH, restRequest.rawPath());

Tuple<String, String> uriParts = splitUri(restRequest.uri());
String query = uriParts.v2();
if (query.isBlank() == false) {
attributes.addAttribute(AttributeNames.HTTP_REQ_QUERY_PARAMS, query);
}
return attributes;
} else {
return Attributes.EMPTY;
}
Expand Down
53 changes: 45 additions & 8 deletions server/src/test/java/org/opensearch/telemetry/tracing/SpanBuilderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

package org.opensearch.telemetry.tracing;

import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;

import org.opensearch.Version;
import org.opensearch.cluster.node.DiscoveryNode;
import org.opensearch.common.network.NetworkAddress;
Expand All @@ -27,29 +29,64 @@

import java.io.IOException;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import java.util.Map;

public class SpanBuilderTests extends OpenSearchTestCase {

public String uri;

public String expectedSpanName;

public String expectedQueryParams;

public String expectedReqRawPath;

@ParametersFactory
public static Collection<Object[]> data() {
return Arrays.asList(
new Object[][] {
{ "/_test/resource?name=John&age=25", "GET /_test/resource", "name=John&age=25", "/_test/resource" },
{ "/_test/", "GET /_test/", "", "/_test/" }, }
);
}

public SpanBuilderTests(String uri, String expectedSpanName, String expectedQueryParams, String expectedReqRawPath) {
this.uri = uri;
this.expectedSpanName = expectedSpanName;
this.expectedQueryParams = expectedQueryParams;
this.expectedReqRawPath = expectedReqRawPath;
}

public void testHttpRequestContext() {
HttpRequest httpRequest = createHttpRequest();
HttpRequest httpRequest = createHttpRequest(uri);
SpanCreationContext context = SpanBuilder.from(httpRequest);
Attributes attributes = context.getAttributes();
assertEquals("GET /_test", context.getSpanName());
assertEquals(expectedSpanName, context.getSpanName());
assertEquals("true", attributes.getAttributesMap().get(AttributeNames.TRACE));
assertEquals("GET", attributes.getAttributesMap().get(AttributeNames.HTTP_METHOD));
assertEquals("HTTP_1_0", attributes.getAttributesMap().get(AttributeNames.HTTP_PROTOCOL_VERSION));
assertEquals("/_test", attributes.getAttributesMap().get(AttributeNames.HTTP_URI));
assertEquals(uri, attributes.getAttributesMap().get(AttributeNames.HTTP_URI));
if (expectedQueryParams.isBlank()) {
assertNull(attributes.getAttributesMap().get(AttributeNames.HTTP_REQ_QUERY_PARAMS));
} else {
assertEquals(expectedQueryParams, attributes.getAttributesMap().get(AttributeNames.HTTP_REQ_QUERY_PARAMS));
}
}

public void testRestRequestContext() {
RestRequest restRequest = RestRequest.request(null, createHttpRequest(), null);
RestRequest restRequest = RestRequest.request(null, createHttpRequest(uri), null);
SpanCreationContext context = SpanBuilder.from(restRequest);
Attributes attributes = context.getAttributes();
assertEquals("GET /_test", context.getSpanName());
assertEquals("/_test", attributes.getAttributesMap().get(AttributeNames.REST_REQ_RAW_PATH));
assertEquals(expectedSpanName, context.getSpanName());
assertEquals(expectedReqRawPath, attributes.getAttributesMap().get(AttributeNames.REST_REQ_RAW_PATH));
assertNotNull(attributes.getAttributesMap().get(AttributeNames.REST_REQ_ID));
if (expectedQueryParams.isBlank()) {
assertNull(attributes.getAttributesMap().get(AttributeNames.HTTP_REQ_QUERY_PARAMS));
} else {
assertEquals(expectedQueryParams, attributes.getAttributesMap().get(AttributeNames.HTTP_REQ_QUERY_PARAMS));
}
}

public void testRestRequestContextForNull() {
Expand Down Expand Up @@ -97,7 +134,7 @@ public void close() {
};
}

private static HttpRequest createHttpRequest() {
private static HttpRequest createHttpRequest(String uri) {
return new HttpRequest() {
@Override
public RestRequest.Method method() {
Expand All @@ -106,7 +143,7 @@ public RestRequest.Method method() {

@Override
public String uri() {
return "/_test";
return uri;
}

@Override
Expand Down
Loading