Skip to content

Commit c21262b

Browse files
danielmitterdorferdanielmitterdorfer
committed
Address review feedback
1 parent f6ae666 commit c21262b

File tree

7 files changed

+78
-112
lines changed

7 files changed

+78
-112
lines changed

‎x-pack/plugin/profiler/src/main/java/org/elasticsearch/xpack/profiler/StackFrame.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,15 @@
77

88
package org.elasticsearch.xpack.profiler;
99

10-
import org.elasticsearch.xcontent.ToXContent;
10+
import org.elasticsearch.xcontent.ObjectPath;
11+
import org.elasticsearch.xcontent.ToXContentObject;
1112
import org.elasticsearch.xcontent.XContentBuilder;
12-
import org.elasticsearch.xpack.profiler.utils.MapExtractor;
1313

1414
import java.io.IOException;
1515
import java.util.Map;
1616
import java.util.Objects;
1717

18-
final class StackFrame implements ToXContent {
18+
final class StackFrame implements ToXContentObject {
1919
String fileName;
2020
String functionName;
2121
Integer functionOffset;
@@ -32,11 +32,11 @@ final class StackFrame implements ToXContent {
3232

3333
public static StackFrame fromSource(Map<String, Object> source) {
3434
return new StackFrame(
35-
MapExtractor.read(source, "Stackframe", "file", "name"),
36-
MapExtractor.read(source, "Stackframe", "function", "name"),
37-
MapExtractor.read(source, "Stackframe", "function", "offset"),
38-
MapExtractor.read(source, "Stackframe", "line", "number"),
39-
MapExtractor.read(source, "Stackframe", "source", "type")
35+
ObjectPath.eval("Stackframe.file.name", source),
36+
ObjectPath.eval("Stackframe.function.name", source),
37+
ObjectPath.eval("Stackframe.function.offset", source),
38+
ObjectPath.eval("Stackframe.line.number", source),
39+
ObjectPath.eval("Stackframe.source.type", source)
4040
);
4141
}
4242

‎x-pack/plugin/profiler/src/main/java/org/elasticsearch/xpack/profiler/StackTrace.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,15 @@
77

88
package org.elasticsearch.xpack.profiler;
99

10-
import org.elasticsearch.xcontent.ToXContent;
10+
import org.elasticsearch.xcontent.ObjectPath;
11+
import org.elasticsearch.xcontent.ToXContentObject;
1112
import org.elasticsearch.xcontent.XContentBuilder;
12-
import org.elasticsearch.xpack.profiler.utils.MapExtractor;
1313

1414
import java.io.IOException;
1515
import java.util.Arrays;
1616
import java.util.Map;
1717

18-
final class StackTrace implements ToXContent {
18+
final class StackTrace implements ToXContentObject {
1919
int[] addressOrLines;
2020
String[] fileIds;
2121
String[] frameIds;
@@ -164,8 +164,8 @@ static String getFileIDFromStackFrameID(String frameID) {
164164
}
165165

166166
public static StackTrace fromSource(Map<String, Object> source) {
167-
String inputFrameIDs = MapExtractor.read(source, "Stacktrace", "frame", "ids");
168-
String inputFrameTypes = MapExtractor.read(source, "Stacktrace", "frame", "types");
167+
String inputFrameIDs = ObjectPath.eval("Stacktrace.frame.ids", source);
168+
String inputFrameTypes = ObjectPath.eval("Stacktrace.frame.types", source);
169169
int countsFrameIDs = inputFrameIDs.length() / BASE64_FRAME_ID_LENGTH;
170170

171171
String[] fileIDs = new String[countsFrameIDs];

‎x-pack/plugin/profiler/src/main/java/org/elasticsearch/xpack/profiler/TransportGetProfilingAction.java‎

Lines changed: 40 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
import org.elasticsearch.action.search.SearchResponse;
1313
import org.elasticsearch.action.support.ActionFilters;
1414
import org.elasticsearch.action.support.HandledTransportAction;
15+
import org.elasticsearch.client.internal.Client;
16+
import org.elasticsearch.client.internal.ParentTaskAssigningClient;
1517
import org.elasticsearch.client.internal.node.NodeClient;
1618
import org.elasticsearch.common.inject.Inject;
1719
import org.elasticsearch.common.util.Maps;
@@ -22,7 +24,7 @@
2224
import org.elasticsearch.search.aggregations.metrics.SumAggregationBuilder;
2325
import org.elasticsearch.tasks.Task;
2426
import org.elasticsearch.transport.TransportService;
25-
import org.elasticsearch.xpack.profiler.utils.MapExtractor;
27+
import org.elasticsearch.xcontent.ObjectPath;
2628

2729
import java.util.Arrays;
2830
import java.util.HashMap;
@@ -34,19 +36,22 @@
3436
public class TransportGetProfilingAction extends HandledTransportAction<GetProfilingRequest, GetProfilingResponse> {
3537
private final NodeClient nodeClient;
3638
private final ThreadContext threadContext;
39+
private final TransportService transportService;
3740

3841
@Inject
3942
public TransportGetProfilingAction(TransportService transportService, ActionFilters actionFilters, NodeClient nodeClient) {
4043
super(GetProfilingAction.NAME, transportService, actionFilters, GetProfilingRequest::new);
4144
this.nodeClient = nodeClient;
45+
this.transportService = transportService;
4246
this.threadContext = transportService.getThreadPool().getThreadContext();
4347
}
4448

4549
@Override
4650
protected void doExecute(Task submitTask, GetProfilingRequest request, ActionListener<GetProfilingResponse> submitListener) {
4751
try (var ignored = threadContext.newTraceContext()) {
52+
Client client = new ParentTaskAssigningClient(this.nodeClient, transportService.getLocalNode(), submitTask);
4853
EventsIndex mediumDownsampled = EventsIndex.MEDIUM_DOWNSAMPLED;
49-
this.nodeClient.prepareSearch(mediumDownsampled.getName())
54+
client.prepareSearch(mediumDownsampled.getName())
5055
.setSize(0)
5156
.setQuery(request.getQuery())
5257
.setTrackTotalHits(true)
@@ -55,7 +60,7 @@ protected void doExecute(Task submitTask, GetProfilingRequest request, ActionLis
5560
public void onResponse(SearchResponse searchResponse) {
5661
long sampleCount = searchResponse.getHits().getTotalHits().value;
5762
EventsIndex resampledIndex = mediumDownsampled.getResampledIndex(request.getSampleSize(), sampleCount);
58-
searchEventGroupByStackTrace(request, resampledIndex, submitListener);
63+
searchEventGroupByStackTrace(client, request, resampledIndex, submitListener);
5964
}
6065

6166
@Override
@@ -67,12 +72,13 @@ public void onFailure(Exception e) {
6772
}
6873

6974
private void searchEventGroupByStackTrace(
75+
Client client,
7076
GetProfilingRequest request,
7177
EventsIndex eventsIndex,
7278
ActionListener<GetProfilingResponse> submitListener
7379
) {
7480
GetProfilingResponseBuilder responseBuilder = new GetProfilingResponseBuilder();
75-
this.nodeClient.prepareSearch(eventsIndex.getName())
81+
client.prepareSearch(eventsIndex.getName())
7682
.setTrackTotalHits(false)
7783
.setQuery(request.getQuery())
7884
.addAggregation(
@@ -86,7 +92,6 @@ private void searchEventGroupByStackTrace(
8692
.subAggregation(new SumAggregationBuilder("count").field("Stacktrace.count"))
8793
)
8894
.addAggregation(new SumAggregationBuilder("total_count").field("Stacktrace.count"))
89-
.setPreFilterShardSize(1)
9095
.execute(new ActionListener<>() {
9196
@Override
9297
public void onResponse(SearchResponse searchResponse) {
@@ -104,7 +109,7 @@ public void onResponse(SearchResponse searchResponse) {
104109
}
105110
if (stackTraceEvents.isEmpty() == false) {
106111
responseBuilder.setStackTraceEvents(stackTraceEvents);
107-
retrieveStackTraces(responseBuilder, submitListener);
112+
retrieveStackTraces(client, responseBuilder, submitListener);
108113
} else {
109114
submitListener.onResponse(responseBuilder.build());
110115
}
@@ -117,8 +122,12 @@ public void onFailure(Exception e) {
117122
});
118123
}
119124

120-
private void retrieveStackTraces(GetProfilingResponseBuilder responseBuilder, ActionListener<GetProfilingResponse> submitListener) {
121-
this.nodeClient.prepareMultiGet()
125+
private void retrieveStackTraces(
126+
Client client,
127+
GetProfilingResponseBuilder responseBuilder,
128+
ActionListener<GetProfilingResponse> submitListener
129+
) {
130+
client.prepareMultiGet()
122131
.addIds("profiling-stacktraces", responseBuilder.getStackTraceEvents().keySet())
123132
.setRealtime(true)
124133
.execute(new ActionListener<>() {
@@ -140,7 +149,7 @@ public void onResponse(MultiGetResponse multiGetItemResponses) {
140149
}
141150
responseBuilder.setStackTraces(stackTracePerId);
142151
responseBuilder.setTotalFrames(totalFrames);
143-
retrieveStackTraceDetails(responseBuilder, stackFrameIds, executableIds, submitListener);
152+
retrieveStackTraceDetails(client, responseBuilder, stackFrameIds, executableIds, submitListener);
144153
}
145154

146155
@Override
@@ -151,6 +160,7 @@ public void onFailure(Exception e) {
151160
}
152161

153162
private void retrieveStackTraceDetails(
163+
Client client,
154164
GetProfilingResponseBuilder responseBuilder,
155165
Set<String> stackFrameIds,
156166
Set<String> executableIds,
@@ -162,40 +172,33 @@ private void retrieveStackTraceDetails(
162172
if (stackFrameIds.isEmpty()) {
163173
handler.onStackFramesResponse(new MultiGetResponse(new MultiGetItemResponse[0]));
164174
} else {
175+
client.prepareMultiGet().addIds("profiling-stackframes", stackFrameIds).setRealtime(true).execute(new ActionListener<>() {
176+
@Override
177+
public void onResponse(MultiGetResponse multiGetItemResponses) {
178+
handler.onStackFramesResponse(multiGetItemResponses);
179+
}
165180

166-
this.nodeClient.prepareMultiGet()
167-
.addIds("profiling-stackframes", stackFrameIds)
168-
.setRealtime(true)
169-
.execute(new ActionListener<>() {
170-
@Override
171-
public void onResponse(MultiGetResponse multiGetItemResponses) {
172-
handler.onStackFramesResponse(multiGetItemResponses);
173-
}
174-
175-
@Override
176-
public void onFailure(Exception e) {
177-
submitListener.onFailure(e);
178-
}
179-
});
181+
@Override
182+
public void onFailure(Exception e) {
183+
submitListener.onFailure(e);
184+
}
185+
});
180186
}
181187
// no data dependency - we can do this concurrently
182188
if (executableIds.isEmpty()) {
183189
handler.onExecutableDetailsResponse(new MultiGetResponse(new MultiGetItemResponse[0]));
184190
} else {
185-
this.nodeClient.prepareMultiGet()
186-
.addIds("profiling-executables", executableIds)
187-
.setRealtime(true)
188-
.execute(new ActionListener<>() {
189-
@Override
190-
public void onResponse(MultiGetResponse multiGetItemResponses) {
191-
handler.onExecutableDetailsResponse(multiGetItemResponses);
192-
}
191+
client.prepareMultiGet().addIds("profiling-executables", executableIds).setRealtime(true).execute(new ActionListener<>() {
192+
@Override
193+
public void onResponse(MultiGetResponse multiGetItemResponses) {
194+
handler.onExecutableDetailsResponse(multiGetItemResponses);
195+
}
193196

194-
@Override
195-
public void onFailure(Exception e) {
196-
submitListener.onFailure(e);
197-
}
198-
});
197+
@Override
198+
public void onFailure(Exception e) {
199+
submitListener.onFailure(e);
200+
}
201+
});
199202
}
200203
}
201204

@@ -276,10 +279,7 @@ public void onExecutableDetailsResponse(MultiGetResponse multiGetItemResponses)
276279
Map<String, String> executables = new HashMap<>();
277280
for (MultiGetItemResponse executable : multiGetItemResponses) {
278281
if (executable.isFailed() == false && executable.getResponse().isExists()) {
279-
executables.put(
280-
executable.getId(),
281-
MapExtractor.read(executable.getResponse().getSource(), "Executable", "file", "name")
282-
);
282+
executables.put(executable.getId(), ObjectPath.eval("Executable.file.name", executable.getResponse().getSource()));
283283
}
284284
}
285285
// publish to object state only when completely done, otherwise mayFinish() could run twice

‎x-pack/plugin/profiler/src/main/java/org/elasticsearch/xpack/profiler/utils/MapExtractor.java‎

Lines changed: 0 additions & 34 deletions
This file was deleted.

‎x-pack/plugin/profiler/src/test/java/org/elasticsearch/xpack/profiler/StackFrameTests.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
import org.elasticsearch.common.bytes.BytesReference;
1111
import org.elasticsearch.test.ESTestCase;
12+
import org.elasticsearch.test.EqualsHashCodeTestUtils;
1213
import org.elasticsearch.xcontent.ToXContent;
1314
import org.elasticsearch.xcontent.XContentBuilder;
1415
import org.elasticsearch.xcontent.XContentFactory;
@@ -58,4 +59,13 @@ public void testToXContent() throws IOException {
5859

5960
assertToXContentEquivalent(BytesReference.bytes(expectedRequest), BytesReference.bytes(actualRequest), contentType);
6061
}
62+
63+
public void testEquality() {
64+
StackFrame frame = new StackFrame("Main.java", "helloWorld", 31733, 22, 3);
65+
EqualsHashCodeTestUtils.checkEqualsAndHashCode(
66+
frame,
67+
(o -> new StackFrame(o.fileName, o.functionName, o.functionOffset, o.lineNumber, o.sourceType))
68+
);
69+
70+
}
6171
}

‎x-pack/plugin/profiler/src/test/java/org/elasticsearch/xpack/profiler/StackTraceTests.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
import org.elasticsearch.common.bytes.BytesReference;
1111
import org.elasticsearch.test.ESTestCase;
12+
import org.elasticsearch.test.EqualsHashCodeTestUtils;
1213
import org.elasticsearch.xcontent.ToXContent;
1314
import org.elasticsearch.xcontent.XContentBuilder;
1415
import org.elasticsearch.xcontent.XContentFactory;
@@ -97,4 +98,18 @@ public void testToXContent() throws IOException {
9798

9899
assertToXContentEquivalent(BytesReference.bytes(expectedRequest), BytesReference.bytes(actualRequest), contentType);
99100
}
101+
102+
public void testEquality() {
103+
StackTrace stackTrace = new StackTrace(
104+
new int[] { 1027822 },
105+
new String[] { "AAAAAAAAAAUAAAAAAAAB3g" },
106+
new String[] { "AAAAAAAAAAUAAAAAAAAB3gAAAAAAD67u" },
107+
new int[] { 2 }
108+
);
109+
110+
EqualsHashCodeTestUtils.checkEqualsAndHashCode(
111+
stackTrace,
112+
(o -> new StackTrace(o.addressOrLines.clone(), o.fileIds.clone(), o.frameIds.clone(), o.typeIds.clone()))
113+
);
114+
}
100115
}

‎x-pack/plugin/profiler/src/test/java/org/elasticsearch/xpack/profiler/utils/MapExtractorTests.java‎

Lines changed: 0 additions & 25 deletions
This file was deleted.

0 commit comments

Comments
 (0)