Skip to content

Commit

Permalink
Changed TraceConfigz zPage form to use POST request (#1521)
Browse files Browse the repository at this point in the history 
* Removed URLEncoder

* Fixed typo

* Added URLDecoding

* Included comment for string replacement

* Added unit tests for special characters in span names

* Resolved URL decoding issues

* Moved url decoding to parseQueryMap and updated the corresponding unit tests

* Added a README file for zPage quickstart

* Add images for README

* Updated README

* Add frontend images

* Add backend images

* Added our design doc

* Added details on package

* Reworded a few lines

* Moved DESIGN.md to a docs folder and changed gradle config to implementation

* Changed wording regarding HttpServer requirement

* Added zpages folder under docs, resolved broken image links

* Resolved comments for the design md file

* Made a few wording changes

* Wrote a benchmark test for TracezSpanBuckets (#23)

* Scaffolded logic for basic benchmark tests

* Wrote benchmark tests for TracezSpanBuckets

* Updated README with benchmark tests

* Changed the wording slightly

* Updated README file (#25)

* Wrote benchmark tests for TracezDataAggregator (#24)

* Scaffolded logic for basic benchmark tests

* Wrote benchmark tests for TracezSpanBuckets

* Updated README with benchmark tests

* Changed the wording slightly

* Added a set of benchmark tests for TracezDataAggregator

* Modified README formatting

* Changed benchmark test to negate dead code elimination

* Added Javadocs to the TracezDataAggregator benchmark tests

* Removed benchmark results from README and added a param to the TracezDataAggregator benchmark tests

* Update sdk_extensions/zpages/src/jmh/java/io/opentelemetry/sdk/extensions/zpages/TracezDataAggregatorBenchmark.java

Co-authored-by: Anuraag Agrawal <[email protected]>

* Added multiple param values for TracezDataAggregatorBenchmark

* Changed TraceConfigz zPage form submit to use POST request

* Added requestMethod parameter to emitHtml, limited TraceConfig change on POST request only

* Removed duplicate parse function, added test for update on POST request only

* Added separate method for processing request

* Removed unnecessary error check in tests, used try resources for inputstream

Co-authored-by: williamhu99 <[email protected]>
Co-authored-by: William Hu <[email protected]>
Co-authored-by: Anuraag Agrawal <[email protected]>
  • Loading branch information
@wty27 @williamhu99 @anuraaga
4 people authored Aug 12, 2020
1 parent 7cd931c commit ce8cfd4
Show file tree
Hide file tree
Showing 7 changed files with 206 additions and 43 deletions.
4 changes: 0 additions & 4 deletions sdk_extensions/zpages/build.gradle
View file
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
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")) {
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
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
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
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

0 comments on commit ce8cfd4

Please sign in to comment.