Skip to content

Commit

Permalink
[Profiling] Allow wildcards for custom indices (elastic#105372)
Browse files Browse the repository at this point in the history 
Several profiling APIs allow to specify custom event indices with the
`indices` parameter. So far this parameter has allowed only one index
and also disallowed wildcards. With this commit we extend this parameter
to support multiple indices and each index can use wildcards.

* Allow to distinguish whether indices are user-provided

As we set indices now internally in all cases due to wildcard
resolution, we need a means to distinguish whether `indices` have been
provided originally via the API or whether Elasticsearch has set them
internally after wildcard resolution.
  • Loading branch information
@danielmitterdorfer
danielmitterdorfer authored Feb 12, 2024
1 parent f942605 commit 858c614
Show file tree
Hide file tree
Showing 5 changed files with 149 additions and 36 deletions.
6 changes: 3 additions & 3 deletions ...rc/internalClusterTest/java/org/elasticsearch/xpack/profiling/GetStackTracesActionIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public void testGetStackTracesFromAPMWithMatchNoDownsampling() throws Exception
1.0d,
1.0d,
query,
"apm-test-*",
new String[] { "apm-test-*" },
"transaction.profiler_stack_trace_ids",
null,
null,
Expand Down Expand Up @@ -99,7 +99,7 @@ public void testGetStackTracesFromAPMWithMatchAndDownsampling() throws Exception
1.0d,
1.0d,
query,
"apm-test-*",
new String[] { "apm-test-*" },
"transaction.profiler_stack_trace_ids",
null,
null,
Expand Down Expand Up @@ -165,7 +165,7 @@ public void testGetStackTracesFromAPMNoMatch() throws Exception {
1.0d,
1.0d,
query,
"apm-test-*",
new String[] { "apm-test-*" },
"transaction.profiler_stack_trace_ids",
null,
null,
Expand Down
89 changes: 65 additions & 24 deletions ...ugin/profiling/src/main/java/org/elasticsearch/xpack/profiling/GetStackTracesRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@
import org.elasticsearch.xcontent.XContentParser;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
Expand All @@ -32,7 +35,7 @@
/**
* A request to get profiling details
*/
public class GetStackTracesRequest extends ActionRequest implements IndicesRequest {
public class GetStackTracesRequest extends ActionRequest implements IndicesRequest.Replaceable {
public static final ParseField QUERY_FIELD = new ParseField("query");
public static final ParseField SAMPLE_SIZE_FIELD = new ParseField("sample_size");
public static final ParseField INDICES_FIELD = new ParseField("indices");
Expand All @@ -49,7 +52,8 @@ public class GetStackTracesRequest extends ActionRequest implements IndicesReque

private QueryBuilder query;
private int sampleSize;
private String indices;
private String[] indices;
private boolean userProvidedIndices;
private String stackTraceIdsField;
private Double requestedDuration;
private Double awsCostFactor;
Expand All @@ -75,7 +79,7 @@ public GetStackTracesRequest(
Double awsCostFactor,
Double azureCostFactor,
QueryBuilder query,
String indices,
String[] indices,
String stackTraceIdsField,
Double customCO2PerKWH,
Double customDatacenterPUE,
Expand All @@ -89,6 +93,7 @@ public GetStackTracesRequest(
this.azureCostFactor = azureCostFactor;
this.query = query;
this.indices = indices;
this.userProvidedIndices = indices != null && indices.length > 0;
this.stackTraceIdsField = stackTraceIdsField;
this.customCO2PerKWH = customCO2PerKWH;
this.customDatacenterPUE = customDatacenterPUE;
Expand Down Expand Up @@ -142,10 +147,14 @@ public QueryBuilder getQuery() {
return query;
}

public String getIndices() {
public String[] getIndices() {
return indices;
}

public boolean isUserProvidedIndices() {
return userProvidedIndices;
}

public String getStackTraceIdsField() {
return stackTraceIdsField;
}
Expand All @@ -164,8 +173,7 @@ public void parseXContent(XContentParser parser) throws IOException {
if (token != XContentParser.Token.START_OBJECT && (token = parser.nextToken()) != XContentParser.Token.START_OBJECT) {
throw new ParsingException(
parser.getTokenLocation(),
"Expected [" + XContentParser.Token.START_OBJECT + "] but found [" + token + "]",
parser.getTokenLocation()
"Expected [" + XContentParser.Token.START_OBJECT + "] but found [" + token + "]."
);
}

Expand All @@ -175,8 +183,6 @@ public void parseXContent(XContentParser parser) throws IOException {
} else if (token.isValue()) {
if (SAMPLE_SIZE_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
this.sampleSize = parser.intValue();
} else if (INDICES_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
this.indices = parser.text();
} else if (STACKTRACE_IDS_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
this.stackTraceIdsField = parser.text();
} else if (REQUESTED_DURATION_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
Expand All @@ -196,22 +202,21 @@ public void parseXContent(XContentParser parser) throws IOException {
} else if (CUSTOM_COST_PER_CORE_HOUR.match(currentFieldName, parser.getDeprecationHandler())) {
this.customCostPerCoreHour = parser.doubleValue();
} else {
throw new ParsingException(
parser.getTokenLocation(),
"Unknown key for a " + token + " in [" + currentFieldName + "].",
parser.getTokenLocation()
);
throw new ParsingException(parser.getTokenLocation(), "Unknown key for a " + token + " in [" + currentFieldName + "].");
}
} else if (token == XContentParser.Token.START_OBJECT) {
if (QUERY_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
this.query = parseTopLevelQuery(parser);
}
} else if (token == XContentParser.Token.START_ARRAY) {
if (INDICES_FIELD.match(currentFieldName, parser.getDeprecationHandler())) {
this.indices = parseIndices(parser);
this.userProvidedIndices = true;
} else {
throw new ParsingException(parser.getTokenLocation(), "Unexpected token " + token + " in [" + currentFieldName + "].");
}
} else {
throw new ParsingException(
parser.getTokenLocation(),
"Unknown key for a " + token + " in [" + currentFieldName + "].",
parser.getTokenLocation()
);
throw new ParsingException(parser.getTokenLocation(), "Unknown key for a " + token + " in [" + currentFieldName + "].");
}
}

Expand All @@ -221,10 +226,32 @@ public void parseXContent(XContentParser parser) throws IOException {
}
}

private String[] parseIndices(XContentParser parser) throws IOException {
XContentParser.Token token;
List<String> indices = new ArrayList<>();
while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) {
if (token == XContentParser.Token.VALUE_STRING) {
indices.add(parser.text());
} else {
throw new ParsingException(
parser.getTokenLocation(),
"Expected ["
+ XContentParser.Token.VALUE_STRING
+ "] but found ["
+ token
+ "] in ["
+ INDICES_FIELD.getPreferredName()
+ "]."
);
}
}
return indices.toArray(new String[0]);
}

@Override
public ActionRequestValidationException validate() {
ActionRequestValidationException validationException = null;
if (indices != null) {
if (userProvidedIndices) {
if (stackTraceIdsField == null || stackTraceIdsField.isEmpty()) {
validationException = addValidationError(
"[" + STACKTRACE_IDS_FIELD.getPreferredName() + "] is mandatory",
Expand Down Expand Up @@ -306,7 +333,7 @@ public boolean equals(Object o) {
GetStackTracesRequest that = (GetStackTracesRequest) o;
return Objects.equals(query, that.query)
&& Objects.equals(sampleSize, that.sampleSize)
&& Objects.equals(indices, that.indices)
&& Arrays.equals(indices, that.indices)
&& Objects.equals(stackTraceIdsField, that.stackTraceIdsField);
}

Expand All @@ -318,7 +345,7 @@ public int hashCode() {
// Resampler to produce a consistent downsampling results, relying on the default hashCode implementation of `query` will
// produce consistent results per node but not across the cluster. To avoid this, we produce the hashCode based on the
// string representation instead, which will produce consistent results for the entire cluster and across node restarts.
return Objects.hash(Objects.toString(query, "null"), sampleSize, indices, stackTraceIdsField);
return Objects.hash(Objects.toString(query, "null"), sampleSize, Arrays.hashCode(indices), stackTraceIdsField);
}

@Override
Expand All @@ -327,10 +354,10 @@ public String[] indices() {
indices.add("profiling-stacktraces");
indices.add("profiling-stackframes");
indices.add("profiling-executables");
if (this.indices == null) {
indices.addAll(EventsIndex.indexNames());
if (userProvidedIndices) {
indices.addAll(List.of(this.indices));
} else {
indices.add(this.indices);
indices.addAll(EventsIndex.indexNames());
}
return indices.toArray(new String[0]);
}
Expand All @@ -344,4 +371,18 @@ public IndicesOptions indicesOptions() {
public boolean includeDataStreams() {
return true;
}

@Override
public IndicesRequest indices(String... indices) {
validateIndices(indices);
this.indices = indices;
return null;
}

private static void validateIndices(String... indices) {
Objects.requireNonNull(indices, "indices must not be null");
for (String index : indices) {
Objects.requireNonNull(index, "index must not be null");
}
}
}
6 changes: 3 additions & 3 deletions ...filing/src/main/java/org/elasticsearch/xpack/profiling/TransportGetStackTracesAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,10 @@ protected void doExecute(Task submitTask, GetStackTracesRequest request, ActionL
licenseChecker.requireSupportedLicense();
GetStackTracesResponseBuilder responseBuilder = new GetStackTracesResponseBuilder(request);
Client client = new ParentTaskAssigningClient(this.nodeClient, transportService.getLocalNode(), submitTask);
if (request.getIndices() == null) {
searchProfilingEvents(submitTask, client, request, submitListener, responseBuilder);
} else {
if (request.isUserProvidedIndices()) {
searchGenericEvents(submitTask, client, request, submitListener, responseBuilder);
} else {
searchProfilingEvents(submitTask, client, request, submitListener, responseBuilder);
}
}

Expand Down
82 changes: 77 additions & 5 deletions ...profiling/src/test/java/org/elasticsearch/xpack/profiling/GetStackTracesRequestTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public void testParseValidXContentWithCustomIndex() throws IOException {
//tag::noformat
.startObject()
.field("sample_size", 2000)
.field("indices", "my-traces")
.field("indices", new String[] {"my-traces"})
.field("stacktrace_ids_field", "stacktraces")
.startObject("query")
.startObject("range")
Expand All @@ -83,7 +83,7 @@ public void testParseValidXContentWithCustomIndex() throws IOException {
request.parseXContent(content);

assertEquals(2000, request.getSampleSize());
assertEquals("my-traces", request.getIndices());
assertArrayEquals(new String[] { "my-traces" }, request.getIndices());
assertEquals("stacktraces", request.getStackTraceIdsField());
// a basic check suffices here
assertEquals("@timestamp", ((RangeQueryBuilder) request.getQuery()).fieldName());
Expand Down Expand Up @@ -169,6 +169,78 @@ public void testParseXContentUnrecognizedField() throws IOException {
}
}

public void testParseXContentCustomIndexNoArray() throws IOException {
try (XContentParser content = createParser(XContentFactory.jsonBuilder()
//tag::noformat
.startObject()
.field("sample_size", 2000)
.field("indices", "my-traces")
.field("stacktrace_ids_field", "stacktraces")
.startObject("query")
.startObject("range")
.startObject("@timestamp")
.field("gte", "2022-10-05")
.endObject()
.endObject()
.endObject()
.endObject()
//end::noformat
)) {

GetStackTracesRequest request = new GetStackTracesRequest();
ParsingException ex = expectThrows(ParsingException.class, () -> request.parseXContent(content));
assertEquals("Unknown key for a VALUE_STRING in [indices].", ex.getMessage());
}
}

public void testParseXContentCustomIndexNullValues() throws IOException {
try (XContentParser content = createParser(XContentFactory.jsonBuilder()
//tag::noformat
.startObject()
.field("sample_size", 2000)
.field("indices", new String[] {"my-traces", null})
.field("stacktrace_ids_field", "stacktraces")
.startObject("query")
.startObject("range")
.startObject("@timestamp")
.field("gte", "2022-10-05")
.endObject()
.endObject()
.endObject()
.endObject()
//end::noformat
)) {

GetStackTracesRequest request = new GetStackTracesRequest();
ParsingException ex = expectThrows(ParsingException.class, () -> request.parseXContent(content));
assertEquals("Expected [VALUE_STRING] but found [VALUE_NULL] in [indices].", ex.getMessage());
}
}

public void testParseXContentCustomIndexInvalidTypes() throws IOException {
try (XContentParser content = createParser(XContentFactory.jsonBuilder()
//tag::noformat
.startObject()
.field("sample_size", 2000)
.field("indices", new int[] {1, 2, 3})
.field("stacktrace_ids_field", "stacktraces")
.startObject("query")
.startObject("range")
.startObject("@timestamp")
.field("gte", "2022-10-05")
.endObject()
.endObject()
.endObject()
.endObject()
//end::noformat
)) {

GetStackTracesRequest request = new GetStackTracesRequest();
ParsingException ex = expectThrows(ParsingException.class, () -> request.parseXContent(content));
assertEquals("Expected [VALUE_STRING] but found [VALUE_NUMBER] in [indices].", ex.getMessage());
}
}

public void testValidateWrongSampleSize() {
GetStackTracesRequest request = new GetStackTracesRequest(
randomIntBetween(Integer.MIN_VALUE, 0),
Expand Down Expand Up @@ -196,7 +268,7 @@ public void testValidateSampleSizeIsValidWithCustomIndices() {
1.0d,
1.0d,
null,
randomAlphaOfLength(7),
new String[] { randomAlphaOfLength(7) },
randomAlphaOfLength(3),
null,
null,
Expand Down Expand Up @@ -234,7 +306,7 @@ public void testValidateIndicesWithoutStacktraces() {
1.0d,
1.0d,
null,
randomAlphaOfLength(5),
new String[] { randomAlphaOfLength(5) },
randomFrom("", null),
null,
null,
Expand All @@ -255,7 +327,7 @@ public void testConsidersCustomIndicesInRelatedIndices() {
1.0d,
1.0d,
null,
customIndex,
new String[] { customIndex },
randomAlphaOfLength(3),
null,
null,
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/profiling/10_basic.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ teardown:
body: >
{
"sample_size": 20000,
"indices": "test-events",
"indices": ["test-event*"],
"stacktrace_ids_field": "events",
"requested_duration": 86400,
"query": {
Expand Down

0 comments on commit 858c614

Please sign in to comment.