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

Changed TraceConfigz zPage form to use POST request #1521

Merged
merged 36 commits into from
Aug 12, 2020
Merged

Changed TraceConfigz zPage form to use POST request #1521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
ec31e3e
Removed URLEncoder
williamhu99 Jul 17, 2020
e0fe0ed
Fixed typo
williamhu99 Jul 18, 2020
cf7f5c8
Added URLDecoding
williamhu99 Jul 20, 2020
9db8e7e
Included comment for string replacement
williamhu99 Jul 20, 2020
4127978
Added unit tests for special characters in span names
williamhu99 Jul 20, 2020
e17c337
Resolved URL decoding issues
williamhu99 Jul 21, 2020
bb58b26
Moved url decoding to parseQueryMap and updated the corresponding uni…
williamhu99 Jul 21, 2020
80a5ccf
Added a README file for zPage quickstart
williamhu99 Jul 22, 2020
dcbd9aa
Add images for README
williamhu99 Jul 22, 2020
24ffce2
Updated README
williamhu99 Jul 22, 2020
5047849
Add frontend images
williamhu99 Jul 22, 2020
85a539e
Add backend images
williamhu99 Jul 22, 2020
4156123
Added our design doc
williamhu99 Jul 22, 2020
265cf27
Added details on package
wty27 Jul 23, 2020
c102e45
Reworded a few lines
williamhu99 Jul 23, 2020
e977dcf
Merge branch 'master' of https://github.com/open-telemetry/openteleme…
williamhu99 Jul 23, 2020
ed8ef33
Moved DESIGN.md to a docs folder and changed gradle config to impleme…
williamhu99 Jul 24, 2020
f595909
Changed wording regarding HttpServer requirement
wty27 Jul 27, 2020
806b632
Added zpages folder under docs, resolved broken image links
wty27 Jul 27, 2020
9e4dde8
Resolved comments for the design md file
williamhu99 Jul 27, 2020
86c2a51
Made a few wording changes
williamhu99 Jul 27, 2020
a3c9a5a
Wrote a benchmark test for TracezSpanBuckets (#23)
williamhu99 Jul 29, 2020
07b2fcf
Updated README file (#25)
wty27 Aug 4, 2020
542f756
Wrote benchmark tests for TracezDataAggregator (#24)
williamhu99 Aug 4, 2020
fe056e3
Merged with original repo
williamhu99 Aug 4, 2020
f714b43
Added Javadocs to the TracezDataAggregator benchmark tests
williamhu99 Aug 5, 2020
922e950
Removed benchmark results from README and added a param to the Tracez…
williamhu99 Aug 5, 2020
13d82c4
Update sdk_extensions/zpages/src/jmh/java/io/opentelemetry/sdk/extens…
williamhu99 Aug 6, 2020
5ab6f6a
Added multiple param values for TracezDataAggregatorBenchmark
williamhu99 Aug 6, 2020
18a4141
Merge branch 'master' of github.com:williamhu99/opentelemetry-java in…
williamhu99 Aug 6, 2020
0ad0ebe
Merged with OpenTelemetry repo for latest updates
wty27 Aug 7, 2020
48138f7
Changed TraceConfigz zPage form submit to use POST request
wty27 Aug 7, 2020
1eb5332
Added requestMethod parameter to emitHtml, limited TraceConfig change…
wty27 Aug 7, 2020
0f09288
Removed duplicate parse function, added test for update on POST reque…
wty27 Aug 10, 2020
8044ba1
Added separate method for processing request
wty27 Aug 11, 2020
95ff5b7
Removed unnecessary error check in tests, used try resources for inpu…
wty27 Aug 11, 2020
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
4 changes: 0 additions & 4 deletions sdk_extensions/zpages/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@ dependencies {

compileOnly 'com.sun.net.httpserver:http:20070405'

annotationProcessor libraries.auto_value

testAnnotationProcessor libraries.auto_value

signature "org.codehaus.mojo.signature:java17:1.0@signature"
}

Expand Down
44 changes: 40 additions & 4 deletions ...zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/TraceConfigzZPageHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,15 +274,15 @@ private void emitHtmlBody(PrintStream out) {
+ ZPageLogo.getLogoBase64()
+ "\" /></a>");
out.print("<h1>Trace Configuration</h1>");
out.print("<form class=\"form-flex\" action=\"" + TRACE_CONFIGZ_URL + "\" method=\"get\">");
out.print("<form class=\"form-flex\" action=\"" + TRACE_CONFIGZ_URL + "\" method=\"post\">");
out.print(
"<input type=\"hidden\" name=\"action\" value=\"" + QUERY_STRING_ACTION_CHANGE + "\" />");
emitChangeTable(out);
// Button for submit
out.print("<button class=\"button\" type=\"submit\" value=\"Submit\">Submit</button>");
out.print("</form>");
// Button for restore default
out.print("<form class=\"form-flex\" action=\"" + TRACE_CONFIGZ_URL + "\" method=\"get\">");
out.print("<form class=\"form-flex\" action=\"" + TRACE_CONFIGZ_URL + "\" method=\"post\">");
out.print(
"<input type=\"hidden\" name=\"action\" value=\"" + QUERY_STRING_ACTION_DEFAULT + "\" />");
out.print("<button class=\"button\" type=\"submit\" value=\"Submit\">Restore Default</button>");
Expand Down Expand Up @@ -313,8 +313,6 @@ public void emitHtml(Map<String, String> queryMap, OutputStream outputStream) {
out.print("</head>");
out.print("<body>");
try {
// Apply updated trace configuration based on query parameters
applyTraceConfig(queryMap);
emitHtmlBody(out);
} catch (Throwable t) {
out.print("Error while generating HTML: " + t.toString());
Expand All @@ -327,6 +325,44 @@ public void emitHtml(Map<String, String> queryMap, OutputStream outputStream) {
}
}

@Override
public boolean processRequest(
String requestMethod, Map<String, String> queryMap, OutputStream outputStream) {
if (requestMethod.equalsIgnoreCase("POST")) {
try {
applyTraceConfig(queryMap);
} catch (Throwable t) {
try (PrintStream out = new PrintStream(outputStream, /* autoFlush= */ false, "UTF-8")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This section seems like an error applying the config, not an error generating HTML right?

Copy link
Author

Choose a reason for hiding this comment

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

Yea it's error applying config, I will adjust the error message

out.print("<!DOCTYPE html>");
out.print("<html lang=\"en\">");
out.print("<head>");
out.print("<meta charset=\"UTF-8\">");
out.print(
"<link rel=\"shortcut icon\" href=\"data:image/png;base64,"
+ ZPageLogo.getFaviconBase64()
+ "\" type=\"image/png\">");
out.print(
"<link href=\"https://fonts.googleapis.com/css?family=Open+Sans:300\""
+ "rel=\"stylesheet\">");
out.print(
"<link href=\"https://fonts.googleapis.com/css?family=Roboto\" rel=\"stylesheet\">");
out.print("<title>" + TRACE_CONFIGZ_NAME + "</title>");
out.print("</head>");
out.print("<body>");
out.print("Error while applying trace config changes: " + t.toString());
out.print("</body>");
out.print("</html>");
logger.log(Level.WARNING, "error while applying trace config changes", t);
} catch (Throwable e) {
logger.log(Level.WARNING, "error while applying trace config changes", e);
return true;
}
return true;
}
}
return false;
}

/**
* Apply updated trace configuration through the tracerProvider based on query parameters.
*
Expand Down
13 changes: 13 additions & 0 deletions sdk_extensions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/ZPageHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,19 @@ public abstract class ZPageHandler {
*/
public abstract String getPageDescription();

/**
* Process requests that require changes (POST/PUT/DELETE).
*
* @param requestMethod the request method HttpHandler received.
* @param queryMap the map of the URL query parameters.
* @return true if theres an error while processing the request.
*/
public boolean processRequest(
String requestMethod, Map<String, String> queryMap, OutputStream outputStream) {
// base no-op method
return false;
}

/**
* Emits the generated HTML page to the {@code outputStream}.
*
Expand Down
37 changes: 27 additions & 10 deletions ...ensions/zpages/src/main/java/io/opentelemetry/sdk/extensions/zpages/ZPageHttpHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableMap;
import com.google.common.io.CharStreams;
import com.sun.net.httpserver.HttpExchange;
import com.sun.net.httpserver.HttpHandler;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.UnsupportedEncodingException;
import java.net.URI;
import java.net.URLDecoder;
import java.util.HashMap;
import java.util.List;
Expand All @@ -47,19 +48,19 @@ final class ZPageHttpHandler implements HttpHandler {
}

/**
* Build a query map from the {@code uri}.
* Build a query map from the query string.
*
* @param uri the {@link URI} for buiding the query map
* @return the query map built based on the @{code uri}
* @param queryString the query string for buiding the query map.
* @return the query map built based on the query string.
*/
@VisibleForTesting
static ImmutableMap<String, String> parseQueryMap(URI uri) throws UnsupportedEncodingException {
String queryStrings = uri.getRawQuery();
if (queryStrings == null) {
static ImmutableMap<String, String> parseQueryString(String queryString)
throws UnsupportedEncodingException {
if (queryString == null) {
return ImmutableMap.of();
}
Map<String, String> queryMap = new HashMap<String, String>();
for (String param : QUERY_SPLITTER.split(queryStrings)) {
for (String param : QUERY_SPLITTER.split(queryString)) {
List<String> keyValuePair = QUERY_KEYVAL_SPLITTER.splitToList(param);
if (keyValuePair.size() > 1) {
if (keyValuePair.get(0).equals(PARAM_SPAN_NAME)) {
Expand All @@ -75,9 +76,25 @@ static ImmutableMap<String, String> parseQueryMap(URI uri) throws UnsupportedEnc
@Override
public final void handle(HttpExchange httpExchange) throws IOException {
try {
String requestMethod = httpExchange.getRequestMethod();
httpExchange.sendResponseHeaders(200, 0);
zpageHandler.emitHtml(
parseQueryMap(httpExchange.getRequestURI()), httpExchange.getResponseBody());
if (requestMethod.equalsIgnoreCase("GET")) {
zpageHandler.emitHtml(
parseQueryString(httpExchange.getRequestURI().getRawQuery()),
httpExchange.getResponseBody());
} else {
final String queryString;
try (InputStreamReader requestBodyReader =
new InputStreamReader(httpExchange.getRequestBody(), "utf-8")) {
queryString = CharStreams.toString(requestBodyReader);
}
boolean error =
zpageHandler.processRequest(
requestMethod, parseQueryString(queryString), httpExchange.getResponseBody());
if (!error) {
zpageHandler.emitHtml(parseQueryString(queryString), httpExchange.getResponseBody());
}
}
} finally {
httpExchange.close();
}
Expand Down
127 changes: 111 additions & 16 deletions ...es/src/test/java/io/opentelemetry/sdk/extensions/zpages/TraceConfigzZPageHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,39 @@
import java.io.ByteArrayOutputStream;
import java.io.OutputStream;
import java.util.Map;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

/** Unit tests for {@link TraceConfigzZPageHandler}. */
public final class TraceConfigzZPageHandlerTest {
private static final TracerSdkProvider tracerProvider = OpenTelemetrySdk.getTracerProvider();
private static final Map<String, String> emptyQueryMap = ImmutableMap.of();

@BeforeEach
void setup() {
// Restore default config
OutputStream output = new ByteArrayOutputStream();
Map<String, String> queryMap = ImmutableMap.of("action", "default");

TraceConfigzZPageHandler traceConfigzZPageHandler =
new TraceConfigzZPageHandler(tracerProvider);
traceConfigzZPageHandler.processRequest("POST", queryMap, output);
traceConfigzZPageHandler.emitHtml(queryMap, output);

assertThat(tracerProvider.getActiveTraceConfig().getSampler().getDescription())
.isEqualTo(TraceConfig.getDefault().getSampler().getDescription());
assertThat(tracerProvider.getActiveTraceConfig().getMaxNumberOfAttributes())
.isEqualTo(TraceConfig.getDefault().getMaxNumberOfAttributes());
assertThat(tracerProvider.getActiveTraceConfig().getMaxNumberOfEvents())
.isEqualTo(TraceConfig.getDefault().getMaxNumberOfEvents());
assertThat(tracerProvider.getActiveTraceConfig().getMaxNumberOfLinks())
.isEqualTo(TraceConfig.getDefault().getMaxNumberOfLinks());
assertThat(tracerProvider.getActiveTraceConfig().getMaxNumberOfAttributesPerEvent())
.isEqualTo(TraceConfig.getDefault().getMaxNumberOfAttributesPerEvent());
assertThat(tracerProvider.getActiveTraceConfig().getMaxNumberOfAttributesPerLink())
.isEqualTo(TraceConfig.getDefault().getMaxNumberOfAttributesPerLink());
}

@Test
void changeTable_emitRowsCorrectly() {
OutputStream output = new ByteArrayOutputStream();
Expand Down Expand Up @@ -154,6 +180,7 @@ void appliesChangesCorrectly_formSubmit() {

TraceConfigzZPageHandler traceConfigzZPageHandler =
new TraceConfigzZPageHandler(tracerProvider);
traceConfigzZPageHandler.processRequest("POST", queryMap, output);
traceConfigzZPageHandler.emitHtml(queryMap, output);

assertThat(tracerProvider.getActiveTraceConfig().getSampler().getDescription())
Expand All @@ -179,6 +206,7 @@ void appliesChangesCorrectly_restoreDefault() {

TraceConfigzZPageHandler traceConfigzZPageHandler =
new TraceConfigzZPageHandler(tracerProvider);
traceConfigzZPageHandler.processRequest("POST", queryMap, output);
traceConfigzZPageHandler.emitHtml(queryMap, output);

assertThat(tracerProvider.getActiveTraceConfig().getSampler().getDescription())
Expand All @@ -203,6 +231,7 @@ void appliesChangesCorrectly_doNotCrashOnNullParameters() {

TraceConfigzZPageHandler traceConfigzZPageHandler =
new TraceConfigzZPageHandler(tracerProvider);
traceConfigzZPageHandler.processRequest("POST", queryMap, output);
traceConfigzZPageHandler.emitHtml(queryMap, output);

assertThat(tracerProvider.getActiveTraceConfig().getSampler().getDescription())
Expand All @@ -228,69 +257,69 @@ void applyChanges_emitErrorOnInvalidInput() {
Map<String, String> queryMap =
ImmutableMap.of("action", "change", "samplingprobability", "invalid");

traceConfigzZPageHandler.emitHtml(queryMap, output);
traceConfigzZPageHandler.processRequest("POST", queryMap, output);

assertThat(output.toString()).contains("Error while generating HTML: ");
assertThat(output.toString()).contains("Error while applying trace config changes: ");
assertThat(output.toString()).contains("SamplingProbability must be of the type double");

// Invalid samplingProbability (< 0)
output = new ByteArrayOutputStream();
traceConfigzZPageHandler = new TraceConfigzZPageHandler(tracerProvider);
queryMap = ImmutableMap.of("action", "change", "samplingprobability", "-1");

traceConfigzZPageHandler.emitHtml(queryMap, output);
traceConfigzZPageHandler.processRequest("POST", queryMap, output);

assertThat(output.toString()).contains("Error while generating HTML: ");
assertThat(output.toString()).contains("Error while applying trace config changes: ");
assertThat(output.toString()).contains("probability must be in range [0.0, 1.0]");

// Invalid samplingProbability (> 1)
output = new ByteArrayOutputStream();
traceConfigzZPageHandler = new TraceConfigzZPageHandler(tracerProvider);
queryMap = ImmutableMap.of("action", "change", "samplingprobability", "1.1");

traceConfigzZPageHandler.emitHtml(queryMap, output);
traceConfigzZPageHandler.processRequest("POST", queryMap, output);

assertThat(output.toString()).contains("Error while generating HTML: ");
assertThat(output.toString()).contains("Error while applying trace config changes: ");
assertThat(output.toString()).contains("probability must be in range [0.0, 1.0]");

// Invalid maxNumOfAttributes
output = new ByteArrayOutputStream();
traceConfigzZPageHandler = new TraceConfigzZPageHandler(tracerProvider);
queryMap = ImmutableMap.of("action", "change", "maxnumofattributes", "invalid");

traceConfigzZPageHandler.emitHtml(queryMap, output);
traceConfigzZPageHandler.processRequest("POST", queryMap, output);

assertThat(output.toString()).contains("Error while generating HTML: ");
assertThat(output.toString()).contains("Error while applying trace config changes: ");
assertThat(output.toString()).contains("MaxNumOfAttributes must be of the type integer");

// Invalid maxNumOfEvents
output = new ByteArrayOutputStream();
traceConfigzZPageHandler = new TraceConfigzZPageHandler(tracerProvider);
queryMap = ImmutableMap.of("action", "change", "maxnumofevents", "invalid");

traceConfigzZPageHandler.emitHtml(queryMap, output);
traceConfigzZPageHandler.processRequest("POST", queryMap, output);

assertThat(output.toString()).contains("Error while generating HTML: ");
assertThat(output.toString()).contains("Error while applying trace config changes: ");
assertThat(output.toString()).contains("MaxNumOfEvents must be of the type integer");

// Invalid maxNumLinks
output = new ByteArrayOutputStream();
traceConfigzZPageHandler = new TraceConfigzZPageHandler(tracerProvider);
queryMap = ImmutableMap.of("action", "change", "maxnumoflinks", "invalid");

traceConfigzZPageHandler.emitHtml(queryMap, output);
traceConfigzZPageHandler.processRequest("POST", queryMap, output);

assertThat(output.toString()).contains("Error while generating HTML: ");
assertThat(output.toString()).contains("Error while applying trace config changes: ");
assertThat(output.toString()).contains("MaxNumOfLinks must be of the type integer");

// Invalid maxNumOfAttributesPerEvent
output = new ByteArrayOutputStream();
traceConfigzZPageHandler = new TraceConfigzZPageHandler(tracerProvider);
queryMap = ImmutableMap.of("action", "change", "maxnumofattributesperevent", "invalid");

traceConfigzZPageHandler.emitHtml(queryMap, output);
traceConfigzZPageHandler.processRequest("POST", queryMap, output);

assertThat(output.toString()).contains("Error while generating HTML: ");
assertThat(output.toString()).contains("Error while applying trace config changes: ");
assertThat(output.toString())
.contains("MaxNumOfAttributesPerEvent must be of the type integer");

Expand All @@ -299,9 +328,75 @@ void applyChanges_emitErrorOnInvalidInput() {
traceConfigzZPageHandler = new TraceConfigzZPageHandler(tracerProvider);
queryMap = ImmutableMap.of("action", "change", "maxnumofattributesperlink", "invalid");

traceConfigzZPageHandler.emitHtml(queryMap, output);
traceConfigzZPageHandler.processRequest("POST", queryMap, output);

assertThat(output.toString()).contains("Error while generating HTML: ");
assertThat(output.toString()).contains("Error while applying trace config changes: ");
assertThat(output.toString()).contains("MaxNumOfAttributesPerLink must be of the type integer");
}

@Test
void applyChanges_shouldNotUpdateOnGetRequest() {
OutputStream output = new ByteArrayOutputStream();
String querySamplingProbability = "samplingprobability";
String queryMaxNumOfAttributes = "maxnumofattributes";
String queryMaxNumOfEvents = "maxnumofevents";
String queryMaxNumOfLinks = "maxnumoflinks";
String queryMaxNumOfAttributesPerEvent = "maxnumofattributesperevent";
String queryMaxNumOfAttributesPerLink = "maxnumofattributesperlink";
String newSamplingProbability = "0.001";
String newMaxNumOfAttributes = "16";
String newMaxNumOfEvents = "16";
String newMaxNumOfLinks = "16";
String newMaxNumOfAttributesPerEvent = "16";
String newMaxNumOfAttributesPerLink = "16";

// Apply new config
Map<String, String> queryMap =
new ImmutableMap.Builder<String, String>()
.put("action", "change")
.put(querySamplingProbability, newSamplingProbability)
.put(queryMaxNumOfAttributes, newMaxNumOfAttributes)
.put(queryMaxNumOfEvents, newMaxNumOfEvents)
.put(queryMaxNumOfLinks, newMaxNumOfLinks)
.put(queryMaxNumOfAttributesPerEvent, newMaxNumOfAttributesPerEvent)
.put(queryMaxNumOfAttributesPerLink, newMaxNumOfAttributesPerLink)
.build();

TraceConfigzZPageHandler traceConfigzZPageHandler =
new TraceConfigzZPageHandler(tracerProvider);

// GET request, Should not apply changes
traceConfigzZPageHandler.emitHtml(queryMap, output);

assertThat(tracerProvider.getActiveTraceConfig().getSampler().getDescription())
.isEqualTo(TraceConfig.getDefault().getSampler().getDescription());
assertThat(tracerProvider.getActiveTraceConfig().getMaxNumberOfAttributes())
.isEqualTo(TraceConfig.getDefault().getMaxNumberOfAttributes());
assertThat(tracerProvider.getActiveTraceConfig().getMaxNumberOfEvents())
.isEqualTo(TraceConfig.getDefault().getMaxNumberOfEvents());
assertThat(tracerProvider.getActiveTraceConfig().getMaxNumberOfLinks())
.isEqualTo(TraceConfig.getDefault().getMaxNumberOfLinks());
assertThat(tracerProvider.getActiveTraceConfig().getMaxNumberOfAttributesPerEvent())
.isEqualTo(TraceConfig.getDefault().getMaxNumberOfAttributesPerEvent());
assertThat(tracerProvider.getActiveTraceConfig().getMaxNumberOfAttributesPerLink())
.isEqualTo(TraceConfig.getDefault().getMaxNumberOfAttributesPerLink());

// POST request, Should apply changes
traceConfigzZPageHandler.processRequest("POST", queryMap, output);
traceConfigzZPageHandler.emitHtml(queryMap, output);

assertThat(tracerProvider.getActiveTraceConfig().getSampler().getDescription())
.isEqualTo(
Samplers.probability(Double.parseDouble(newSamplingProbability)).getDescription());
assertThat(tracerProvider.getActiveTraceConfig().getMaxNumberOfAttributes())
.isEqualTo(Integer.parseInt(newMaxNumOfAttributes));
assertThat(tracerProvider.getActiveTraceConfig().getMaxNumberOfEvents())
.isEqualTo(Integer.parseInt(newMaxNumOfEvents));
assertThat(tracerProvider.getActiveTraceConfig().getMaxNumberOfLinks())
.isEqualTo(Integer.parseInt(newMaxNumOfLinks));
assertThat(tracerProvider.getActiveTraceConfig().getMaxNumberOfAttributesPerEvent())
.isEqualTo(Integer.parseInt(newMaxNumOfAttributesPerEvent));
assertThat(tracerProvider.getActiveTraceConfig().getMaxNumberOfAttributesPerLink())
.isEqualTo(Integer.parseInt(newMaxNumOfAttributesPerLink));
}
}
Loading