diff --git a/README.md b/README.md index 8a9ff52..5c3c788 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,7 @@ Please fill out a bug report [here](https://github.com/Stephenson-Software/gh-ba ## Usage reporting -Usage reporting is on by default: gh-backup reports that it was used to the maintainers' [trace](https://github.com/Stephenson-Software/trace) service at `https://trace.danielstephenson.dev`, sending a `startup` event carrying its name and version once per process, and a `backup-completed` event carrying nothing else when a backup run finishes. Nothing else is sent: nothing about the users, organizations or repositories being backed up, and no usernames, hostnames, IP addresses, paths or command-line arguments. A one-line notice is logged the first time it runs on a machine (recorded in `~/.config/gh-backup/usage-reporting-notice-shown`). +Usage reporting is on by default: gh-backup reports that it was used to the maintainers' [trace](https://github.com/Stephenson-Software/trace) service at `https://trace.danielstephenson.dev`, sending a `startup` event carrying its name and version once per process, and a `backup-completed` event carrying only the version when a backup run finishes. Nothing else is sent: nothing about the users, organizations or repositories being backed up, and no usernames, hostnames, IP addresses, paths or command-line arguments. A one-line notice is logged the first time it runs on a machine (recorded in `~/.config/gh-backup/usage-reporting-notice-shown`). To turn it off, any one of these is enough: diff --git a/src/main/java/com/github/backup/UsageReportingService.java b/src/main/java/com/github/backup/UsageReportingService.java index 13c7e79..68f507b 100644 --- a/src/main/java/com/github/backup/UsageReportingService.java +++ b/src/main/java/com/github/backup/UsageReportingService.java @@ -13,16 +13,15 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.Collections; /** * Reports that gh-backup was used, to the trace service, and never gets in the * way of a backup. * *
Two events are sent, both off the calling thread through the vendored - * {@link TraceClient}: {@code startup} once per process (tagged with the - * program version only) and {@code backup-completed} when a backup run - * finishes, with no tags at all. Nothing identifying is sent: no user or + * {@link TraceClient}: {@code startup} once per process and + * {@code backup-completed} when a backup run finishes, each tagged with the + * program version only. Nothing identifying is sent: no user or * organization names, no repository names, no counts, no paths, no hostnames. * *
Reporting is on by default and switched off with @@ -53,9 +52,10 @@ public class UsageReportingService { static final String NOTICE_MARKER_FILE = "usage-reporting-notice-shown"; /** The public page describing what trace collects and every way to turn it off. */ static final String DETAILS_URL = "https://github.com/Stephenson-Software/trace#usage-reporting"; + /** Sent as the version when the build did not supply one. */ + static final String UNKNOWN_VERSION = "unknown"; private final TraceClient client; - private final String version; private final Path noticeMarker; @Autowired @@ -68,16 +68,24 @@ public UsageReportingService( } UsageReportingService(String enabled, String endpoint, String key, String version, Path noticeMarker) { - this.client = buildClient(enabled, endpoint, key); - this.version = version; + this.client = buildClient(enabled, endpoint, key, versionOrUnknown(version)); this.noticeMarker = noticeMarker; } - private static TraceClient buildClient(String enabled, String endpoint, String key) { + /** + * The program version sent with every event. An unfiltered "@project.version@" means the + * build did not run through Maven; that, or a blank value, is sent as "unknown". + */ + static String versionOrUnknown(String version) { + String trimmed = version == null ? "" : version.trim(); + return trimmed.isEmpty() || trimmed.startsWith("@") ? UNKNOWN_VERSION : trimmed; + } + + private static TraceClient buildClient(String enabled, String endpoint, String key, String version) { // A blank value (an empty environment variable, say) means "default", i.e. on. boolean on = enabled == null || enabled.isBlank() || !"false".equalsIgnoreCase(enabled.trim()); try { - return TraceClient.builder(endpoint, APPLICATION) + return TraceClient.builder(endpoint, APPLICATION, version) .key(key) .enabled(on) .logger(java.util.logging.Logger.getLogger(UsageReportingService.class.getName())) @@ -108,16 +116,10 @@ void start() { return; } showFirstRunNoticeOnce(); - String tagged = version == null ? "" : version.trim(); - // An unfiltered "@project.version@" means the build did not run through Maven; send no tag then. - if (tagged.isEmpty() || tagged.startsWith("@")) { - client.report(STARTUP_EVENT); - } else { - client.report(STARTUP_EVENT, null, Collections.singletonMap("version", tagged)); - } + client.report(STARTUP_EVENT); } - /** Reports that a backup run finished. Carries nothing about what was backed up. */ + /** Reports that a backup run finished. Carries the version only, nothing about what was backed up. */ public void backupCompleted() { client.report(BACKUP_COMPLETED_EVENT); } @@ -141,7 +143,7 @@ private void showFirstRunNoticeOnce() { if (Files.exists(noticeMarker)) { return; } - log.info("Usage reporting is on: gh-backup sends its name and version (a startup event) and a " + log.info("Usage reporting is on: gh-backup sends its name and version with a startup event and a " + "backup-completed event (nothing else) to https://trace.danielstephenson.dev - nothing " + "about accounts, repositories or this machine. Turn it off with " + "-Dusage.reporting.enabled=false, USAGE_REPORTING_ENABLED=false or " diff --git a/src/main/java/com/github/backup/trace/TraceClient.java b/src/main/java/com/github/backup/trace/TraceClient.java index 7f488bd..2ece347 100644 --- a/src/main/java/com/github/backup/trace/TraceClient.java +++ b/src/main/java/com/github/backup/trace/TraceClient.java @@ -1,5 +1,5 @@ /* - * trace-client 0.2.0 -- https://github.com/Stephenson-Software/trace-client-java + * trace-client 0.4.0 -- https://github.com/Stephenson-Software/trace-client-java * * One call to report that a program was used. Copy this file into a project as * is, or depend on the artifact; either way there is nothing else to add. @@ -66,12 +66,24 @@ *
The same server-wide file can also carry a {@code tags:} block, merged + * into every event every plugin on the server reports -- {@code ci: "true"} + * on a test server keeps its events out of real-installation figures. An + * event's own tag wins over a server-wide one of the same name. See + * {@link Builder#serverWideConfig(File)}. + * + *
Every event carries the program's own version as the tag + * {@code version} -- the third argument to {@link #builder}, required, so a + * {@code command} event can be tied to a release as well as a + * {@code startup} one. An event's own {@code version} tag wins over it. + * *
A disabled client is a no-op that costs nothing. Programs that run on * other people's machines should expose their own switch in their * configuration and say on startup whether reporting is on. * *
{@code
- * TraceClient trace = TraceClient.builder("https://trace.example.org", "MyPlugin")
+ * TraceClient trace = TraceClient.builder("https://trace.example.org", "MyPlugin",
+ * getDescription().getVersion())
* .key(config.getString("usage-reporting.key"))
* .enabled(config.getBoolean("usage-reporting.enabled", true))
* .serverWideConfig(getDataFolder().getParentFile()) // plugins/
@@ -93,6 +105,9 @@
*/
public final class TraceClient {
+ /** This client's version, as sent in the User-Agent. */
+ public static final String VERSION = "0.4.0";
+
/** How many reports may wait to be sent before new ones are dropped. */
public static final int QUEUE_CAPACITY = 256;
@@ -123,9 +138,24 @@ public final class TraceClient {
+ "# Set enabled to false and every such plugin on this server stops reporting,\n"
+ "# regardless of its own usage-reporting.enabled setting. Plugins never turn\n"
+ "# this back on.\n"
- + "enabled: true\n";
+ + "enabled: true\n"
+ + "#\n"
+ + "# Tags added to every event such plugins on this server report. A plugin's\n"
+ + "# own tag of the same name wins. On a test or CI server, uncomment the two\n"
+ + "# lines below so its events are left out of real-installation figures.\n"
+ + "# tags:\n"
+ + "# ci: \"true\"\n";
private static final Pattern ENABLED_LINE = Pattern.compile("^\\s*enabled\\s*:\\s*(\\S+)");
+ private static final Pattern TAGS_LINE = Pattern.compile("^tags\\s*:\\s*(#.*)?$");
+
+ // What the trace server accepts in a report's tags (MetricDto): at most
+ // MAX_TAGS pairs, keys not blank, keys and values at most MAX_TAG_LENGTH
+ // characters. Server-wide tags are held to that and to a stricter key
+ // alphabet, so a typo in the file can never turn every report into a 400.
+ static final int MAX_TAGS = 32;
+ static final int MAX_TAG_LENGTH = 255;
+ private static final Pattern TAG_KEY = Pattern.compile("[A-Za-z0-9][A-Za-z0-9_.\\-]*");
// Where environment variables come from. A seam rather than System.getenv
// directly, so tests can point it at a map; nothing else should touch it.
@@ -134,16 +164,23 @@ public final class TraceClient {
private final String endpoint;
private final String key;
private final String application;
+ private final String version;
private final Logger logger;
private final String disabledReason; // null when enabled
+ private final Map serverWideTags; // never null; read once, at build()
private final ThreadPoolExecutor executor; // null when disabled
private TraceClient(Builder builder) {
this.endpoint = builder.baseUrl.replaceAll("/+$", "") + "/api/metrics";
this.key = builder.key;
this.application = builder.application;
+ this.version = builder.version;
this.logger = builder.logger;
- this.disabledReason = disabledReason(builder);
+ ServerWideConfig serverWide = builder.pluginsDirectory == null || environmentDisables()
+ ? ServerWideConfig.NONE
+ : readServerWideConfig(builder.pluginsDirectory);
+ this.disabledReason = disabledReason(builder, serverWide);
+ this.serverWideTags = serverWide.tags;
if (disabledReason == null) {
this.executor = new ThreadPoolExecutor(
1, 1, 30, TimeUnit.SECONDS,
@@ -162,15 +199,18 @@ private TraceClient(Builder builder) {
/**
* Starts describing a client for the program named {@code application},
- * reporting to the trace server at {@code baseUrl}.
+ * at {@code version}, reporting to the trace server at {@code baseUrl}.
+ * The version is sent as the tag {@code version} on every event; a blank
+ * one, or one longer than {@value #MAX_TAG_LENGTH} characters, is an
+ * {@link IllegalArgumentException}.
*/
- public static Builder builder(String baseUrl, String application) {
- return new Builder(baseUrl, application);
+ public static Builder builder(String baseUrl, String application, String version) {
+ return new Builder(baseUrl, application, version);
}
/** A client that reports nothing. Useful as a default before configuration is read. */
public static TraceClient disabled() {
- return new Builder("http://disabled.invalid", "disabled").enabled(false).build();
+ return new Builder("http://disabled.invalid", "disabled", "disabled").enabled(false).build();
}
/** Whether {@link #report} will actually send anything. */
@@ -188,11 +228,11 @@ public String disabledReason() {
return disabledReason;
}
- private String disabledReason(Builder builder) {
+ private static String disabledReason(Builder builder, ServerWideConfig serverWide) {
if (environmentDisables()) {
return REASON_ENVIRONMENT;
}
- if (builder.pluginsDirectory != null && serverWideConfigDisables(builder.pluginsDirectory)) {
+ if (serverWide.disables) {
return REASON_SERVER_WIDE;
}
if (!builder.enabled) {
@@ -224,33 +264,230 @@ private static boolean isYes(String value) {
return v.equals("1") || v.equals("true") || v.equals("yes");
}
+ /** What {@code plugins/trace/config.yml} says: the switch and the server-wide tags. */
+ static final class ServerWideConfig {
+ static final ServerWideConfig NONE = new ServerWideConfig(false, Collections.emptyMap());
+
+ final boolean disables;
+ final Map tags;
+
+ ServerWideConfig(boolean disables, Map tags) {
+ this.disables = disables;
+ this.tags = tags;
+ }
+ }
+
/**
* Ensures {@code /trace/config.yml} exists and reads its
- * {@code enabled:} line. No YAML library: the file is ours, one key deep,
- * and a line regex is enough. Anything going wrong on disk is logged at
- * FINE and counts as enabled -- a read-only plugins directory must not
- * silently switch reporting off, nor stop the host program.
+ * {@code enabled:} line and {@code tags:} block. No YAML library: the file
+ * is ours, shallow, and a line scan is enough. Anything going wrong on
+ * disk is logged at FINE and counts as enabled with no tags -- a
+ * read-only plugins directory must not silently switch reporting off,
+ * nor stop the host program.
*/
- private boolean serverWideConfigDisables(File pluginsDirectory) {
+ private ServerWideConfig readServerWideConfig(File pluginsDirectory) {
Path file = new File(pluginsDirectory, SERVER_WIDE_CONFIG_PATH).toPath();
try {
if (!Files.exists(file)) {
Files.createDirectories(file.getParent());
Files.write(file, SERVER_WIDE_CONFIG_CONTENT.getBytes(StandardCharsets.UTF_8));
- return false; // just written with enabled: true
+ // just written: enabled: true, and the tags example commented out
+ }
+ return parseServerWideConfig(Files.readAllLines(file, StandardCharsets.UTF_8));
+ } catch (IOException | RuntimeException failure) {
+ log("could not read server-wide config " + file + ": " + failure);
+ return ServerWideConfig.NONE;
+ }
+ }
+
+ /**
+ * Reads the switch and the tags from the lines of the server-wide file.
+ * The first {@code enabled:} line outside a {@code tags:} block is the
+ * switch. A {@code tags:} line at column 0 opens a block of indented
+ * {@code key: value} lines, which ends at the next non-blank line that is
+ * not indented; blank and {@code #} lines inside it are skipped, as are
+ * lines indented differently from its first entry. Values may be bare,
+ * double- or single-quoted. Entries the trace server would reject -- and
+ * anything this reader does not understand -- are dropped, one by one,
+ * and at most {@link #MAX_TAGS} are kept; nothing here throws.
+ */
+ static ServerWideConfig parseServerWideConfig(List lines) {
+ Boolean disables = null;
+ Map tags = new LinkedHashMap<>();
+ boolean inTags = false;
+ int entryIndent = -1;
+ for (String line : lines) {
+ String trimmed = line.trim();
+ if (trimmed.isEmpty() || trimmed.startsWith("#")) {
+ continue;
+ }
+ int indent = 0;
+ while (indent < line.length() && (line.charAt(indent) == ' ' || line.charAt(indent) == '\t')) {
+ indent++;
+ }
+ if (inTags) {
+ if (indent > 0) {
+ if (entryIndent < 0) {
+ entryIndent = indent;
+ }
+ if (indent == entryIndent) {
+ addServerWideTag(tags, trimmed);
+ }
+ continue;
+ }
+ inTags = false;
+ }
+ if (TAGS_LINE.matcher(line).matches()) {
+ inTags = true;
+ entryIndent = -1;
+ continue;
}
- List lines = Files.readAllLines(file, StandardCharsets.UTF_8);
- for (String line : lines) {
+ if (disables == null) {
Matcher matcher = ENABLED_LINE.matcher(line);
if (matcher.find()) {
- return isOff(matcher.group(1));
+ disables = isOff(matcher.group(1));
+ }
+ }
+ }
+ return new ServerWideConfig(disables != null && disables,
+ tags.isEmpty() ? Collections.emptyMap() : Collections.unmodifiableMap(tags));
+ }
+
+ private static void addServerWideTag(Map tags, String entry) {
+ if (tags.size() >= MAX_TAGS) {
+ return;
+ }
+ int colon = entry.indexOf(':');
+ if (colon <= 0) {
+ return;
+ }
+ String key = unquote(entry.substring(0, colon).trim());
+ String rest = entry.substring(colon + 1);
+ if (key == null || !rest.isEmpty() && rest.charAt(0) != ' ' && rest.charAt(0) != '\t') {
+ return; // "a:b" is a string in YAML, not a pair
+ }
+ String value = scalar(rest.trim());
+ if (value == null
+ || key.length() > MAX_TAG_LENGTH || !TAG_KEY.matcher(key).matches()
+ || value.length() > MAX_TAG_LENGTH) {
+ return;
+ }
+ if (!tags.containsKey(key)) {
+ tags.put(key, value);
+ }
+ }
+
+ /** A key, quoted or not; null when the quoting is broken. */
+ private static String unquote(String key) {
+ if (key.startsWith("\"") || key.startsWith("'")) {
+ return key.length() >= 2 && key.charAt(key.length() - 1) == key.charAt(0)
+ ? key.substring(1, key.length() - 1)
+ : null;
+ }
+ return key;
+ }
+
+ /**
+ * A YAML scalar value, with any trailing comment removed; null for an
+ * empty (YAML null) value, a broken quote, or anything that is not a
+ * plain one-line scalar.
+ */
+ private static String scalar(String text) {
+ if (text.isEmpty() || text.startsWith("#")) {
+ return null;
+ }
+ char first = text.charAt(0);
+ if (first == '"' || first == '\'') {
+ StringBuilder out = new StringBuilder();
+ int i = 1;
+ for (; i < text.length(); i++) {
+ char c = text.charAt(i);
+ if (first == '"' && c == '\\' && i + 1 < text.length()) {
+ char next = text.charAt(++i);
+ switch (next) {
+ case 'n': out.append('\n'); break;
+ case 't': out.append('\t'); break;
+ case 'r': out.append('\r'); break;
+ default: out.append(next); // \" \\ \/ and anything else, literally
+ }
+ } else if (c == first) {
+ if (first == '\'' && i + 1 < text.length() && text.charAt(i + 1) == '\'') {
+ out.append('\'');
+ i++;
+ } else {
+ break;
+ }
+ } else {
+ out.append(c);
+ }
+ }
+ if (i >= text.length()) {
+ return null; // never closed
+ }
+ String after = text.substring(i + 1).trim();
+ return after.isEmpty() || after.startsWith("#") ? out.toString() : null;
+ }
+ if ("[{|>&*!%@`".indexOf(first) >= 0) {
+ return null; // flow collections, block scalars, anchors, tags: not ours
+ }
+ int comment = -1;
+ for (int i = 1; i < text.length(); i++) {
+ if (text.charAt(i) == '#' && (text.charAt(i - 1) == ' ' || text.charAt(i - 1) == '\t')) {
+ comment = i;
+ break;
+ }
+ }
+ String value = (comment < 0 ? text : text.substring(0, comment)).trim();
+ return value.isEmpty() ? null : value;
+ }
+
+ /**
+ * The event's tags with the server-wide ones added: an event's own tag
+ * wins on a key conflict, and server-wide tags stop being added once
+ * {@link #MAX_TAGS} is reached, so the merge never makes a report the
+ * server would reject. A tag with a null key or value counts as absent,
+ * as it does in {@link #json}.
+ */
+ static Map withServerWideTags(Map tags, Map serverWide) {
+ if (serverWide.isEmpty()) {
+ return tags;
+ }
+ Map merged = new LinkedHashMap<>();
+ if (tags != null) {
+ for (Map.Entry tag : new LinkedHashMap<>(tags).entrySet()) {
+ if (tag.getKey() != null && tag.getValue() != null) {
+ merged.put(tag.getKey(), tag.getValue());
}
}
- return false; // no enabled: line at all
- } catch (IOException | RuntimeException failure) {
- log("could not read server-wide config " + file + ": " + failure);
- return false;
}
+ for (Map.Entry tag : serverWide.entrySet()) {
+ if (merged.size() >= MAX_TAGS) {
+ break;
+ }
+ if (!merged.containsKey(tag.getKey())) {
+ merged.put(tag.getKey(), tag.getValue());
+ }
+ }
+ return merged;
+ }
+
+ /**
+ * The event's own tags plus {@code version}, unless the event already
+ * carries one. A copy; the caller's map is never modified.
+ */
+ static Map withVersion(Map tags, String version) {
+ Map merged = new LinkedHashMap<>();
+ if (tags != null) {
+ for (Map.Entry tag : new LinkedHashMap<>(tags).entrySet()) {
+ if (tag.getKey() != null && tag.getValue() != null) {
+ merged.put(tag.getKey(), tag.getValue());
+ }
+ }
+ }
+ if (!merged.containsKey("version")) {
+ merged.put("version", version);
+ }
+ return merged;
}
/** Reports that {@code name} happened. */
@@ -266,7 +503,8 @@ public void report(String name, Double value, Map tags) {
if (executor == null || name == null || name.trim().isEmpty()) {
return;
}
- final String body = json(application, name, value, tags);
+ final String body = json(application, name, value,
+ withServerWideTags(withVersion(tags, version), serverWideTags));
executor.execute(() -> send(body));
}
@@ -303,7 +541,7 @@ private void send(String body) {
connection.setRequestMethod("POST");
connection.setRequestProperty("Content-Type", "application/json; charset=utf-8");
connection.setRequestProperty("Authorization", "Bearer " + key);
- connection.setRequestProperty("User-Agent", "trace-client/0.2.0 (" + application + ")");
+ connection.setRequestProperty("User-Agent", "trace-client/" + VERSION + " (" + application + ")");
connection.setDoOutput(true);
byte[] bytes = body.getBytes(StandardCharsets.UTF_8);
connection.setFixedLengthStreamingMode(bytes.length);
@@ -399,20 +637,28 @@ static String quote(String text) {
public static final class Builder {
private final String baseUrl;
private final String application;
+ private final String version;
private String key;
private boolean enabled = true;
private File pluginsDirectory;
private Logger logger;
- private Builder(String baseUrl, String application) {
+ private Builder(String baseUrl, String application, String version) {
if (baseUrl == null || baseUrl.trim().isEmpty()) {
throw new IllegalArgumentException("baseUrl is required");
}
if (application == null || application.trim().isEmpty()) {
throw new IllegalArgumentException("application is required");
}
+ if (version == null || version.trim().isEmpty()) {
+ throw new IllegalArgumentException("version is required");
+ }
+ if (version.trim().length() > MAX_TAG_LENGTH) {
+ throw new IllegalArgumentException("version is longer than " + MAX_TAG_LENGTH + " characters");
+ }
this.baseUrl = baseUrl.trim();
this.application = application.trim();
+ this.version = version.trim();
}
/** The program's write key. Without one the client is a no-op. */
@@ -434,7 +680,17 @@ public Builder enabled(boolean enabled) {
* {@code plugins/trace/config.yml} exists -- creating it with
* {@code enabled: true} if it is missing -- and honours
* {@code enabled: false} in it. The file is never rewritten once it
- * exists. Optional; programs that are not plugins leave it unset.
+ * exists. Its optional {@code tags:} block is added to every event
+ * this client reports, below the event's own tags:
+ *
+ *
+ * enabled: true
+ * tags:
+ * ci: "true"
+ *
+ *
+ * Both are read once, here. Optional; programs that are not
+ * plugins leave it unset.
*/
public Builder serverWideConfig(File pluginsDirectory) {
this.pluginsDirectory = pluginsDirectory;
diff --git a/src/test/java/com/github/backup/UsageReportingServiceTest.java b/src/test/java/com/github/backup/UsageReportingServiceTest.java
index 098d985..571a315 100644
--- a/src/test/java/com/github/backup/UsageReportingServiceTest.java
+++ b/src/test/java/com/github/backup/UsageReportingServiceTest.java
@@ -92,7 +92,7 @@ void startupEventCarriesTheApplicationNameAndVersionOnly() throws Exception {
}
@Test
- void backupCompletedEventCarriesNothingElse() throws Exception {
+ void backupCompletedEventCarriesTheVersionOnly() throws Exception {
UsageReportingService service = new UsageReportingService("true", endpoint(), "test-key", "2.0.0-TEST", marker());
service.start();
assertTrue(arrived.await(5, TimeUnit.SECONDS));
@@ -102,18 +102,18 @@ void backupCompletedEventCarriesNothingElse() throws Exception {
assertTrue(arrived.await(5, TimeUnit.SECONDS), "backup-completed event should arrive");
service.close();
- assertEquals("{\"application\":\"gh-backup\",\"name\":\"backup-completed\"}", bodies.get(1));
+ assertEquals("{\"application\":\"gh-backup\",\"name\":\"backup-completed\",\"tags\":{\"version\":\"2.0.0-TEST\"}}", bodies.get(1));
}
@Test
- void unfilteredVersionPlaceholderIsNotSentAsATag() throws Exception {
+ void unfilteredVersionPlaceholderIsSentAsUnknown() throws Exception {
UsageReportingService service = new UsageReportingService("true", endpoint(), "test-key", "@project.version@", marker());
service.start();
assertTrue(arrived.await(5, TimeUnit.SECONDS));
service.close();
- assertEquals("{\"application\":\"gh-backup\",\"name\":\"startup\"}", bodies.get(0));
+ assertEquals("{\"application\":\"gh-backup\",\"name\":\"startup\",\"tags\":{\"version\":\"unknown\"}}", bodies.get(0));
}
@Test
diff --git a/src/test/java/com/github/backup/trace/TraceClientTest.java b/src/test/java/com/github/backup/trace/TraceClientTest.java
index f614727..59f30b9 100644
--- a/src/test/java/com/github/backup/trace/TraceClientTest.java
+++ b/src/test/java/com/github/backup/trace/TraceClientTest.java
@@ -13,6 +13,7 @@
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
+import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashMap;
@@ -52,13 +53,15 @@ private static final class Received {
final String path;
final String authorization;
final String contentType;
+ final String userAgent;
final String body;
- Received(String method, String path, String authorization, String contentType, String body) {
+ Received(String method, String path, String authorization, String contentType, String userAgent, String body) {
this.method = method;
this.path = path;
this.authorization = authorization;
this.contentType = contentType;
+ this.userAgent = userAgent;
this.body = body;
}
}
@@ -84,6 +87,7 @@ void startServer() throws Exception {
exchange.getRequestURI().getPath(),
exchange.getRequestHeaders().getFirst("Authorization"),
exchange.getRequestHeaders().getFirst("Content-Type"),
+ exchange.getRequestHeaders().getFirst("User-Agent"),
new String(body, StandardCharsets.UTF_8)));
exchange.sendResponseHeaders(replyStatus, -1);
exchange.close();
@@ -104,7 +108,7 @@ private String baseUrl() {
@Test
void report_postsTheEventToTheMetricsEndpointWithTheKey() throws Exception {
// Arrange
- TraceClient client = TraceClient.builder(baseUrl() + "/", "MyPlugin").key("k-123").build();
+ TraceClient client = TraceClient.builder(baseUrl() + "/", "MyPlugin", "1.2.3").key("k-123").build();
// Act
client.report("startup");
@@ -116,14 +120,14 @@ void report_postsTheEventToTheMetricsEndpointWithTheKey() throws Exception {
assertEquals("/api/metrics", request.path, "a trailing slash on the base URL must not double up");
assertEquals("Bearer k-123", request.authorization);
assertTrue(request.contentType.startsWith("application/json"), request.contentType);
- assertEquals("{\"application\":\"MyPlugin\",\"name\":\"startup\"}", request.body);
+ assertEquals("{\"application\":\"MyPlugin\",\"name\":\"startup\",\"tags\":{\"version\":\"1.2.3\"}}", request.body);
client.close();
}
@Test
void report_carriesValueAndTagsWhenGiven() throws Exception {
// Arrange
- TraceClient client = TraceClient.builder(baseUrl(), "MyPlugin").key("k").build();
+ TraceClient client = TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key("k").build();
Map tags = new LinkedHashMap<>();
tags.put("command", "home");
tags.put("world", "the \"end\"");
@@ -135,7 +139,7 @@ void report_carriesValueAndTagsWhenGiven() throws Exception {
assertTrue(arrived.await(5, TimeUnit.SECONDS));
assertEquals(
"{\"application\":\"MyPlugin\",\"name\":\"command\",\"value\":2.5,"
- + "\"tags\":{\"command\":\"home\",\"world\":\"the \\\"end\\\"\"}}",
+ + "\"tags\":{\"command\":\"home\",\"world\":\"the \\\"end\\\"\",\"version\":\"1.2.3\"}}",
received.get(0).body);
client.close();
}
@@ -158,7 +162,7 @@ void report_returnsBeforeTheServerAnswers() throws Exception {
exchange.close();
});
slow.start();
- TraceClient client = TraceClient.builder("http://127.0.0.1:" + slow.getAddress().getPort(), "MyPlugin")
+ TraceClient client = TraceClient.builder("http://127.0.0.1:" + slow.getAddress().getPort(), "MyPlugin", "1.2.3")
.key("k").build();
// Act
@@ -184,7 +188,7 @@ void report_doesNotThrowWhenNothingIsListening() throws Exception {
Logger logger = Logger.getLogger("TraceClientTest.dead");
logger.setLevel(Level.ALL);
logger.addHandler(log);
- TraceClient client = TraceClient.builder("http://127.0.0.1:" + deadPort, "MyPlugin")
+ TraceClient client = TraceClient.builder("http://127.0.0.1:" + deadPort, "MyPlugin", "1.2.3")
.key("k").logger(logger).build();
// Act
@@ -205,7 +209,7 @@ void report_doesNotThrowWhenTheServerRejectsTheKey() throws Exception {
Logger logger = Logger.getLogger("TraceClientTest.rejected");
logger.setLevel(Level.ALL);
logger.addHandler(log);
- TraceClient client = TraceClient.builder(baseUrl(), "MyPlugin").key("revoked").logger(logger).build();
+ TraceClient client = TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key("revoked").logger(logger).build();
// Act
assertDoesNotThrow(() -> client.report("startup"));
@@ -219,9 +223,9 @@ void report_doesNotThrowWhenTheServerRejectsTheKey() throws Exception {
@Test
void disabledClient_sendsNothing() throws Exception {
// Arrange
- TraceClient byFlag = TraceClient.builder(baseUrl(), "MyPlugin").key("k").enabled(false).build();
- TraceClient byMissingKey = TraceClient.builder(baseUrl(), "MyPlugin").build();
- TraceClient byBlankKey = TraceClient.builder(baseUrl(), "MyPlugin").key(" ").build();
+ TraceClient byFlag = TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key("k").enabled(false).build();
+ TraceClient byMissingKey = TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").build();
+ TraceClient byBlankKey = TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key(" ").build();
TraceClient explicit = TraceClient.disabled();
// Act
@@ -239,7 +243,7 @@ void disabledClient_sendsNothing() throws Exception {
@Test
void report_ignoresABlankName() throws Exception {
// Arrange
- TraceClient client = TraceClient.builder(baseUrl(), "MyPlugin").key("k").build();
+ TraceClient client = TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key("k").build();
// Act
client.report(null);
@@ -253,10 +257,66 @@ void report_ignoresABlankName() throws Exception {
@Test
void builder_rejectsAMissingBaseUrlOrApplication() {
- assertThrows(IllegalArgumentException.class, () -> TraceClient.builder(null, "MyPlugin"));
- assertThrows(IllegalArgumentException.class, () -> TraceClient.builder(" ", "MyPlugin"));
- assertThrows(IllegalArgumentException.class, () -> TraceClient.builder("http://x", null));
- assertThrows(IllegalArgumentException.class, () -> TraceClient.builder("http://x", ""));
+ assertThrows(IllegalArgumentException.class, () -> TraceClient.builder(null, "MyPlugin", "1.2.3"));
+ assertThrows(IllegalArgumentException.class, () -> TraceClient.builder(" ", "MyPlugin", "1.2.3"));
+ assertThrows(IllegalArgumentException.class, () -> TraceClient.builder("http://x", null, "1.2.3"));
+ assertThrows(IllegalArgumentException.class, () -> TraceClient.builder("http://x", "", "1.2.3"));
+ }
+
+ @Test
+ void builder_rejectsAMissingOrOverlongVersion() {
+ assertThrows(IllegalArgumentException.class, () -> TraceClient.builder("http://x", "MyPlugin", null));
+ assertThrows(IllegalArgumentException.class, () -> TraceClient.builder("http://x", "MyPlugin", " "));
+ StringBuilder overlong = new StringBuilder();
+ for (int i = 0; i <= TraceClient.MAX_TAG_LENGTH; i++) {
+ overlong.append('9');
+ }
+ assertThrows(IllegalArgumentException.class, () -> TraceClient.builder("http://x", "MyPlugin", overlong.toString()));
+ }
+
+ @Test
+ void report_tagsACommandWithTheProgramVersionTrimmed() throws Exception {
+ // Arrange
+ TraceClient client = TraceClient.builder(baseUrl(), "MyPlugin", " 2.0.0-SNAPSHOT ").key("k").build();
+
+ // Act
+ client.report("command", null, Collections.singletonMap("name", "home"));
+
+ // Assert
+ assertTrue(arrived.await(5, TimeUnit.SECONDS));
+ assertEquals("{\"application\":\"MyPlugin\",\"name\":\"command\","
+ + "\"tags\":{\"name\":\"home\",\"version\":\"2.0.0-SNAPSHOT\"}}", received.get(0).body);
+ client.close();
+ }
+
+ @Test
+ void report_anEventsOwnVersionTagWinsOverTheProgramVersion() throws Exception {
+ // Arrange
+ TraceClient client = TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key("k").build();
+ Map tags = new LinkedHashMap<>();
+ tags.put("version", "9.9.9");
+
+ // Act
+ client.report("startup", null, tags);
+
+ // Assert
+ assertTrue(arrived.await(5, TimeUnit.SECONDS));
+ assertEquals("{\"application\":\"MyPlugin\",\"name\":\"startup\",\"tags\":{\"version\":\"9.9.9\"}}",
+ received.get(0).body);
+ assertEquals(Collections.singletonMap("version", "9.9.9"), tags, "the caller's map is not modified");
+ client.close();
+ }
+
+ @Test
+ void withVersion_neverModifiesTheCallersMap() {
+ Map tags = new LinkedHashMap<>();
+ tags.put("name", "home");
+
+ Map merged = TraceClient.withVersion(tags, "1.2.3");
+
+ assertEquals(Collections.singletonMap("name", "home"), tags);
+ assertEquals("1.2.3", merged.get("version"));
+ assertEquals("1.2.3", TraceClient.withVersion(null, "1.2.3").get("version"));
}
@Test
@@ -273,6 +333,63 @@ void json_escapesControlCharactersAndSkipsNullTags() {
assertEquals("\"\\u0001\"", TraceClient.quote("\u0001"));
}
+ @Test
+ void json_dropsInfiniteValuesAndKeepsFiniteOnes() {
+ // Arrange + Act
+ String positive = TraceClient.json("App", "n", Double.POSITIVE_INFINITY, null);
+ String negative = TraceClient.json("App", "n", Double.NEGATIVE_INFINITY, null);
+ String finite = TraceClient.json("App", "n", -0.5, null);
+
+ // Assert
+ assertEquals("{\"application\":\"App\",\"name\":\"n\"}", positive, "Infinity is not JSON and is dropped");
+ assertEquals("{\"application\":\"App\",\"name\":\"n\"}", negative, "-Infinity is not JSON and is dropped");
+ assertEquals("{\"application\":\"App\",\"name\":\"n\",\"value\":-0.5}", finite);
+ }
+
+ @Test
+ void json_omitsTagsWhenNullOrEmpty() {
+ // Arrange + Act
+ String nullTags = TraceClient.json("App", "n", null, null);
+ String emptyTags = TraceClient.json("App", "n", null, Collections.emptyMap());
+
+ // Assert
+ assertEquals("{\"application\":\"App\",\"name\":\"n\"}", nullTags);
+ assertEquals("{\"application\":\"App\",\"name\":\"n\"}", emptyTags);
+ }
+
+ @Test
+ void json_writesAnEmptyTagsObjectWhenEveryTagIsSkipped() {
+ // Characterizes current behaviour: a map that is non-empty but holds
+ // only null keys or values still produces a "tags" key, with nothing
+ // in it. Valid JSON either way.
+ // Arrange
+ Map tags = new LinkedHashMap<>();
+ tags.put("nullValue", null);
+ tags.put(null, "nullKey");
+
+ // Act
+ String json = TraceClient.json("App", "n", null, tags);
+
+ // Assert
+ assertEquals("{\"application\":\"App\",\"name\":\"n\",\"tags\":{}}", json);
+ }
+
+ @Test
+ void quote_escapesCarriageReturnAndEveryOtherControlCharacter() {
+ assertEquals("\"a\\rb\"", TraceClient.quote("a\rb"));
+ assertEquals("\"\\u0000\"", TraceClient.quote("\u0000"));
+ assertEquals("\"\\u0008\"", TraceClient.quote("\b"));
+ assertEquals("\"\\u000c\"", TraceClient.quote("\f"));
+ assertEquals("\"\\u001f\"", TraceClient.quote("\u001f"));
+ assertEquals("\"\"", TraceClient.quote(""));
+ }
+
+ @Test
+ void quote_passesPrintableAndNonAsciiCharactersThrough() {
+ // Only what JSON requires is escaped; the body is sent as UTF-8.
+ assertEquals("\" /é€\u007f\"", TraceClient.quote(" /é€\u007f"));
+ }
+
@Test
void queue_isBoundedAndDropsRatherThanGrows() throws Exception {
// Arrange
@@ -292,7 +409,7 @@ void queue_isBoundedAndDropsRatherThanGrows() throws Exception {
exchange.close();
});
slow.start();
- TraceClient client = TraceClient.builder("http://127.0.0.1:" + slow.getAddress().getPort(), "MyPlugin")
+ TraceClient client = TraceClient.builder("http://127.0.0.1:" + slow.getAddress().getPort(), "MyPlugin", "1.2.3")
.key("k").build();
int flood = TraceClient.QUEUE_CAPACITY * 3;
@@ -327,7 +444,7 @@ void close_sendsWhatWasJustQueuedBeforeStopping() throws Exception {
// races the sender thread and is lost a good fraction of the time; 30
// back-to-back report()+close() pairs make that fraction visible.
for (int i = 0; i < 30; i++) {
- TraceClient client = TraceClient.builder(baseUrl(), "MyCli").key("k").build();
+ TraceClient client = TraceClient.builder(baseUrl(), "MyCli", "1.2.3").key("k").build();
client.report("startup", null, Collections.singletonMap("run", String.valueOf(i)));
client.close();
}
@@ -344,7 +461,7 @@ void close_stillReturnsWithinTheTimeoutWhenTheServerHangs() throws Exception {
exchange.close();
});
slow.start();
- TraceClient client = TraceClient.builder("http://127.0.0.1:" + slow.getAddress().getPort(), "MyCli").key("k").build();
+ TraceClient client = TraceClient.builder("http://127.0.0.1:" + slow.getAddress().getPort(), "MyCli", "1.2.3").key("k").build();
client.report("startup");
long before = System.nanoTime();
@@ -356,12 +473,95 @@ void close_stillReturnsWithinTheTimeoutWhenTheServerHangs() throws Exception {
slow.stop(0);
}
+ @Test
+ void close_isSafeToCallTwice() throws Exception {
+ // Arrange
+ TraceClient client = TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key("k").build();
+ client.report("startup");
+
+ // Act
+ client.close();
+
+ // Assert
+ assertDoesNotThrow(client::close);
+ assertEquals(1, received.size(), "the report queued before the first close() is delivered once");
+ }
+
+ @Test
+ void report_afterCloseIsDroppedWithoutThrowing() throws Exception {
+ // Arrange
+ TraceClient client = TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key("k").build();
+ client.close();
+
+ // Act
+ assertDoesNotThrow(() -> client.report("late"));
+
+ // Assert
+ assertFalse(arrived.await(300, TimeUnit.MILLISECONDS), "nothing should be sent after close()");
+ assertTrue(received.isEmpty());
+ }
+
+ @Test
+ void report_sendsAUserAgentNamingTheClientAndTheApplication() throws Exception {
+ // Arrange
+ TraceClient client = TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key("k").build();
+
+ // Act
+ client.report("startup");
+
+ // Assert
+ assertTrue(arrived.await(5, TimeUnit.SECONDS));
+ String userAgent = received.get(0).userAgent;
+ assertTrue(userAgent != null && userAgent.matches("trace-client/\\d+\\.\\d+\\.\\d+ \\(MyPlugin\\)"),
+ "unexpected User-Agent: " + userAgent);
+ client.close();
+ }
+
+ @Test
+ void report_logsASuccessStatusOtherThan201() throws Exception {
+ // Arrange
+ // The server answers 201 Created; any other status, even a 2xx, is
+ // worth a FINE line because it means the endpoint is not what the
+ // client expects.
+ replyStatus = 200;
+ RecordingHandler log = new RecordingHandler();
+ Logger logger = Logger.getLogger("TraceClientTest.status200");
+ logger.setLevel(Level.ALL);
+ logger.addHandler(log);
+ TraceClient client = TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key("k").logger(logger).build();
+
+ // Act
+ assertDoesNotThrow(() -> client.report("startup"));
+
+ // Assert
+ assertTrue(log.await(5, TimeUnit.SECONDS));
+ assertEquals(Level.FINE, log.records.get(0).getLevel());
+ assertTrue(log.records.get(0).getMessage().startsWith("[trace] trace server answered 200"),
+ log.records.get(0).getMessage());
+ client.close();
+ }
+
+ @Test
+ void builder_trimsTheBaseUrlAndApplicationAndDropsEveryTrailingSlash() throws Exception {
+ // Arrange
+ TraceClient client = TraceClient.builder(" " + baseUrl() + "/// ", " MyPlugin ", "1.2.3").key("k").build();
+
+ // Act
+ client.report("startup");
+
+ // Assert
+ assertTrue(arrived.await(5, TimeUnit.SECONDS));
+ assertEquals("/api/metrics", received.get(0).path);
+ assertEquals("{\"application\":\"MyPlugin\",\"name\":\"startup\",\"tags\":{\"version\":\"1.2.3\"}}", received.get(0).body);
+ client.close();
+ }
+
@Test
void disabledClient_saysWhy() {
- assertEquals(TraceClient.REASON_CONFIG, TraceClient.builder(baseUrl(), "MyPlugin").key("k").enabled(false).build().disabledReason());
- assertEquals(TraceClient.REASON_NO_KEY, TraceClient.builder(baseUrl(), "MyPlugin").build().disabledReason());
- assertEquals(TraceClient.REASON_NO_KEY, TraceClient.builder(baseUrl(), "MyPlugin").key(" ").build().disabledReason());
- assertNull(TraceClient.builder(baseUrl(), "MyPlugin").key("k").build().disabledReason(), "an enabled client has no reason");
+ assertEquals(TraceClient.REASON_CONFIG, TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key("k").enabled(false).build().disabledReason());
+ assertEquals(TraceClient.REASON_NO_KEY, TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").build().disabledReason());
+ assertEquals(TraceClient.REASON_NO_KEY, TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key(" ").build().disabledReason());
+ assertNull(TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key("k").build().disabledReason(), "an enabled client has no reason");
}
@Test
@@ -372,7 +572,7 @@ void serverWideConfig_isCreatedWithTheExactContentWhenMissing(@TempDir Path plug
assertFalse(Files.exists(file));
// Act
- TraceClient client = TraceClient.builder(baseUrl(), "MyPlugin").key("k")
+ TraceClient client = TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key("k")
.serverWideConfig(pluginsDirectory).build();
// Assert
@@ -382,7 +582,13 @@ void serverWideConfig_isCreatedWithTheExactContentWhenMissing(@TempDir Path plug
+ "# Set enabled to false and every such plugin on this server stops reporting,\n"
+ "# regardless of its own usage-reporting.enabled setting. Plugins never turn\n"
+ "# this back on.\n"
- + "enabled: true\n";
+ + "enabled: true\n"
+ + "#\n"
+ + "# Tags added to every event such plugins on this server report. A plugin's\n"
+ + "# own tag of the same name wins. On a test or CI server, uncomment the two\n"
+ + "# lines below so its events are left out of real-installation figures.\n"
+ + "# tags:\n"
+ + "# ci: \"true\"\n";
assertEquals(expected, new String(Files.readAllBytes(file), StandardCharsets.UTF_8));
assertTrue(client.isEnabled(), "a freshly created switch file means enabled");
assertNull(client.disabledReason());
@@ -398,7 +604,7 @@ void serverWideConfig_enabledFalseDisablesWithTheServerWideReason(@TempDir Path
Files.write(file, operatorsFile.getBytes(StandardCharsets.UTF_8));
// Act
- TraceClient client = TraceClient.builder(baseUrl(), "MyPlugin").key("k").enabled(true)
+ TraceClient client = TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key("k").enabled(true)
.serverWideConfig(plugins.toFile()).build();
client.report("startup");
client.close();
@@ -418,16 +624,16 @@ void serverWideConfig_acceptsEverySpellingOfOff(@TempDir Path plugins) throws Ex
for (String off : new String[] {"false", "no", "0", "off", "OFF", "No"}) {
Files.write(file, ("enabled: " + off + "\n").getBytes(StandardCharsets.UTF_8));
assertEquals(TraceClient.REASON_SERVER_WIDE,
- TraceClient.builder(baseUrl(), "MyPlugin").key("k").serverWideConfig(plugins.toFile()).build().disabledReason(),
+ TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key("k").serverWideConfig(plugins.toFile()).build().disabledReason(),
"enabled: " + off + " should disable");
}
for (String on : new String[] {"true", "yes", "1", "on", "anything-else"}) {
Files.write(file, ("enabled: " + on + "\n").getBytes(StandardCharsets.UTF_8));
- assertNull(TraceClient.builder(baseUrl(), "MyPlugin").key("k").serverWideConfig(plugins.toFile()).build().disabledReason(),
+ assertNull(TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key("k").serverWideConfig(plugins.toFile()).build().disabledReason(),
"enabled: " + on + " should not disable");
}
Files.write(file, "# nothing here\n".getBytes(StandardCharsets.UTF_8));
- assertNull(TraceClient.builder(baseUrl(), "MyPlugin").key("k").serverWideConfig(plugins.toFile()).build().disabledReason(),
+ assertNull(TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key("k").serverWideConfig(plugins.toFile()).build().disabledReason(),
"a file without an enabled: line means enabled");
}
@@ -436,13 +642,13 @@ void environment_disablesAndWinsOverTheServerWideFile(@TempDir Path plugins) thr
// Arrange
// The file says on; the environment says off. The environment wins,
// and is the reason given.
- TraceClient.builder(baseUrl(), "MyPlugin").key("k").serverWideConfig(plugins.toFile()).build().close();
- assertEquals("enabled: true\n", lastLine(plugins.resolve("trace").resolve("config.yml")));
+ TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key("k").serverWideConfig(plugins.toFile()).build().close();
+ assertTrue(Files.readAllLines(plugins.resolve("trace").resolve("config.yml"), StandardCharsets.UTF_8).contains("enabled: true"));
for (String off : new String[] {"off", "OFF", "false", "0", "no", " No "}) {
environment.clear();
environment.put("TRACE_USAGE_REPORTING", off);
- TraceClient client = TraceClient.builder(baseUrl(), "MyPlugin").key("k").serverWideConfig(plugins.toFile()).build();
+ TraceClient client = TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key("k").serverWideConfig(plugins.toFile()).build();
assertFalse(client.isEnabled(), "TRACE_USAGE_REPORTING=" + off + " should disable");
assertEquals("environment", client.disabledReason());
client.report("startup");
@@ -451,7 +657,7 @@ void environment_disablesAndWinsOverTheServerWideFile(@TempDir Path plugins) thr
for (String yes : new String[] {"1", "true", "TRUE", "yes"}) {
environment.clear();
environment.put("DO_NOT_TRACK", yes);
- TraceClient client = TraceClient.builder(baseUrl(), "MyPlugin").key("k").serverWideConfig(plugins.toFile()).build();
+ TraceClient client = TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key("k").serverWideConfig(plugins.toFile()).build();
assertFalse(client.isEnabled(), "DO_NOT_TRACK=" + yes + " should disable");
assertEquals("environment", client.disabledReason());
client.report("startup");
@@ -463,7 +669,7 @@ void environment_disablesAndWinsOverTheServerWideFile(@TempDir Path plugins) thr
environment.clear();
environment.put("TRACE_USAGE_REPORTING", "on");
environment.put("DO_NOT_TRACK", "0");
- assertNull(TraceClient.builder(baseUrl(), "MyPlugin").key("k").serverWideConfig(plugins.toFile()).build().disabledReason());
+ assertNull(TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key("k").serverWideConfig(plugins.toFile()).build().disabledReason());
}
@Test
@@ -477,21 +683,252 @@ void disabledReason_followsThePrecedenceEnvironmentThenServerWideThenConfigThenK
// Act + Assert: peel the reasons off one at a time, in order.
assertEquals("environment",
- TraceClient.builder(baseUrl(), "MyPlugin").enabled(false).serverWideConfig(pluginsDirectory).build().disabledReason());
+ TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").enabled(false).serverWideConfig(pluginsDirectory).build().disabledReason());
environment.clear();
assertEquals("server-wide config: plugins/trace/config.yml",
- TraceClient.builder(baseUrl(), "MyPlugin").enabled(false).serverWideConfig(pluginsDirectory).build().disabledReason());
+ TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").enabled(false).serverWideConfig(pluginsDirectory).build().disabledReason());
Files.write(file, "enabled: true\n".getBytes(StandardCharsets.UTF_8));
assertEquals("config.yml",
- TraceClient.builder(baseUrl(), "MyPlugin").enabled(false).serverWideConfig(pluginsDirectory).build().disabledReason());
+ TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").enabled(false).serverWideConfig(pluginsDirectory).build().disabledReason());
assertEquals("no key",
- TraceClient.builder(baseUrl(), "MyPlugin").enabled(true).serverWideConfig(pluginsDirectory).build().disabledReason());
- TraceClient enabled = TraceClient.builder(baseUrl(), "MyPlugin").key("k").enabled(true).serverWideConfig(pluginsDirectory).build();
+ TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").enabled(true).serverWideConfig(pluginsDirectory).build().disabledReason());
+ TraceClient enabled = TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key("k").enabled(true).serverWideConfig(pluginsDirectory).build();
assertNull(enabled.disabledReason());
assertTrue(enabled.isEnabled());
enabled.close();
}
+ @Test
+ void serverWideTags_areMergedIntoEveryEvent(@TempDir Path plugins) throws Exception {
+ // Arrange
+ writeServerWideConfig(plugins, "enabled: true\ntags:\n ci: \"true\"\n");
+ Map tags = new LinkedHashMap<>();
+ tags.put("version", "1.2.3");
+
+ // Act
+ String body = reportedBody(plugins, "startup", tags);
+
+ // Assert
+ assertEquals("{\"application\":\"MyPlugin\",\"name\":\"startup\",\"tags\":{\"version\":\"1.2.3\",\"ci\":\"true\"}}", body);
+ }
+
+ @Test
+ void serverWideTags_areAddedToAnEventWithNoTagsOfItsOwn(@TempDir Path plugins) throws Exception {
+ writeServerWideConfig(plugins, "tags:\n ci: \"true\"\n");
+
+ assertEquals("{\"application\":\"MyPlugin\",\"name\":\"startup\",\"tags\":{\"version\":\"1.2.3\",\"ci\":\"true\"}}",
+ reportedBody(plugins, "startup", null));
+ }
+
+ @Test
+ void serverWideTags_neverOverwriteTheEventsOwnTag(@TempDir Path plugins) throws Exception {
+ // Arrange
+ writeServerWideConfig(plugins, "tags:\n version: \"9.9.9\"\n name: overwritten\n ci: true\n");
+ Map tags = new LinkedHashMap<>();
+ tags.put("version", "1.2.3");
+ tags.put("name", "home");
+
+ // Act
+ String body = reportedBody(plugins, "command", tags);
+
+ // Assert
+ assertEquals("{\"application\":\"MyPlugin\",\"name\":\"command\",\"tags\":"
+ + "{\"version\":\"1.2.3\",\"name\":\"home\",\"ci\":\"true\"}}", body);
+ }
+
+ @Test
+ void serverWideTags_acceptDoubleQuotedSingleQuotedAndBareValues() {
+ Map tags = tagsOf("tags:\n"
+ + " a: \"double \\\"quoted\\\" # not a comment\"\n"
+ + " b: 'single ''quoted'''\n"
+ + " c: bare value # a comment\n"
+ + " d: true\n"
+ + " e: \"\"\n"
+ + " 'f': \"quoted key\"\n"
+ + " g: \"x\" # comment after a quote\n");
+ Map expected = new LinkedHashMap<>();
+ expected.put("a", "double \"quoted\" # not a comment");
+ expected.put("b", "single 'quoted'");
+ expected.put("c", "bare value");
+ expected.put("d", "true");
+ expected.put("e", "");
+ expected.put("f", "quoted key");
+ expected.put("g", "x");
+ assertEquals(expected, tags);
+ }
+
+ @Test
+ void serverWideTags_skipBlankAndCommentLinesInsideTheBlock() {
+ Map tags = tagsOf("tags: # server-wide\n"
+ + "\n"
+ + " # the CI marker\n"
+ + "# a comment at column 0 does not end the block either\n"
+ + " ci: \"true\"\n"
+ + " \n"
+ + " env: staging\n");
+ Map expected = new LinkedHashMap<>();
+ expected.put("ci", "true");
+ expected.put("env", "staging");
+ assertEquals(expected, tags);
+ }
+
+ @Test
+ void serverWideTags_blockEndsAtTheNextUnindentedLineOrEndOfFile() {
+ // Ends at the next key.
+ TraceClient.ServerWideConfig config = TraceClient.parseServerWideConfig(Arrays.asList(
+ "tags:", " ci: \"true\"", "enabled: false", " stray: x"));
+ assertEquals(Collections.singletonMap("ci", "true"), config.tags);
+ assertTrue(config.disables, "the enabled: line after the block is still the switch");
+
+ // Ends at end of file, with no trailing newline.
+ assertEquals(Collections.singletonMap("ci", "true"), tagsOf("enabled: true\ntags:\n ci: \"true\""));
+
+ // An enabled: entry inside the block is a tag, not the switch.
+ config = TraceClient.parseServerWideConfig(Arrays.asList("tags:", " enabled: \"false\""));
+ assertFalse(config.disables);
+ assertEquals(Collections.singletonMap("enabled", "false"), config.tags);
+
+ // Deeper-indented lines are not entries of this block.
+ assertEquals(Collections.singletonMap("ci", "true"), tagsOf("tags:\n ci: \"true\"\n nested: x\n"));
+
+ // "tags:" must be at column 0, and an empty block is no tags.
+ assertTrue(tagsOf("other:\n tags:\n ci: \"true\"\n").isEmpty());
+ assertTrue(tagsOf("tags:\nenabled: true\n").isEmpty());
+ }
+
+ @Test
+ void serverWideTags_dropEntriesTheServerWouldRejectOrThatAreNotUnderstood() {
+ StringBuilder longText = new StringBuilder();
+ for (int i = 0; i < TraceClient.MAX_TAG_LENGTH + 1; i++) {
+ longText.append('x');
+ }
+ String exactlyMax = longText.substring(1);
+ Map tags = tagsOf("tags:\n"
+ + " ci: \"true\"\n"
+ + " " + longText + ": key-too-long\n"
+ + " long: \"" + longText + "\"\n"
+ + " max: " + exactlyMax + "\n"
+ + " \"has space\": x\n"
+ + " \"\": blank-key\n"
+ + " -dash-first: x\n"
+ + " empty:\n"
+ + " comment-only: # nothing\n"
+ + " a:b\n"
+ + " no colon at all\n"
+ + " broken: \"never closed\n"
+ + " trailing: \"x\" junk\n"
+ + " list: [1, 2]\n"
+ + " map: {a: b}\n"
+ + " block: |\n"
+ + " ok.key_1-2: fine\n"
+ + " ci: \"duplicate, first wins\"\n");
+ Map expected = new LinkedHashMap<>();
+ expected.put("ci", "true");
+ expected.put("max", exactlyMax);
+ expected.put("ok.key_1-2", "fine");
+ assertEquals(expected, tags);
+ }
+
+ @Test
+ void serverWideTags_areCappedSoTheEventStaysWithinTheServersTagLimit(@TempDir Path plugins) throws Exception {
+ // Arrange: 40 server-wide tags, 30 event tags.
+ StringBuilder file = new StringBuilder("tags:\n");
+ for (int i = 0; i < 40; i++) {
+ file.append(" s").append(i).append(": v\n");
+ }
+ assertEquals(TraceClient.MAX_TAGS, tagsOf(file.toString()).size(), "at most MAX_TAGS are read");
+ writeServerWideConfig(plugins, file.toString());
+ Map tags = new LinkedHashMap<>();
+ for (int i = 0; i < 30; i++) {
+ tags.put("e" + i, "v");
+ }
+
+ // Act
+ String body = reportedBody(plugins, "startup", tags);
+
+ // Assert
+ int pairs = body.split("\":\"v\"", -1).length - 1;
+ assertEquals(TraceClient.MAX_TAGS - 1, pairs, "30 event tags + version + 1 server-wide: " + body);
+ assertTrue(body.contains("\"e29\":\"v\"") && body.contains("\"version\":\"1.2.3\"")
+ && body.contains("\"s0\":\"v\"") && !body.contains("\"s1\""), body);
+ }
+
+ @Test
+ void serverWideTags_doNotResurrectADisabledClient(@TempDir Path plugins) throws Exception {
+ writeServerWideConfig(plugins, "enabled: false\ntags:\n ci: \"true\"\n");
+
+ TraceClient client = TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key("k").serverWideConfig(plugins.toFile()).build();
+ client.report("startup");
+ client.close();
+
+ assertFalse(client.isEnabled());
+ assertEquals(TraceClient.REASON_SERVER_WIDE, client.disabledReason());
+ assertFalse(arrived.await(300, TimeUnit.MILLISECONDS), "nothing should have been sent");
+ }
+
+ @Test
+ void serverWideTags_areNoneWithoutAServerWideConfigOrAFileThatHasNone(@TempDir Path plugins) throws Exception {
+ // No serverWideConfig(...) at all.
+ TraceClient client = TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key("k").build();
+ client.report("startup");
+ assertTrue(arrived.await(5, TimeUnit.SECONDS));
+ client.close();
+ assertEquals("{\"application\":\"MyPlugin\",\"name\":\"startup\",\"tags\":{\"version\":\"1.2.3\"}}", received.get(0).body);
+
+ // A file with only the switch in it.
+ arrived = new CountDownLatch(1);
+ writeServerWideConfig(plugins, "enabled: true\n");
+ assertEquals("{\"application\":\"MyPlugin\",\"name\":\"startup\",\"tags\":{\"version\":\"1.2.3\"}}", reportedBody(plugins, "startup", null));
+ }
+
+ @Test
+ void serverWideTags_aFreshlyCreatedFileHasNoActiveTags(@TempDir Path plugins) throws Exception {
+ // Created by build() because it was missing ...
+ String body = reportedBody(plugins, "startup", null);
+
+ // ... and nothing in it is live: the example is commented out.
+ assertTrue(Files.exists(plugins.resolve("trace").resolve("config.yml")));
+ assertEquals("{\"application\":\"MyPlugin\",\"name\":\"startup\",\"tags\":{\"version\":\"1.2.3\"}}", body);
+ TraceClient.ServerWideConfig config = TraceClient.parseServerWideConfig(
+ Arrays.asList(TraceClient.SERVER_WIDE_CONFIG_CONTENT.split("\n")));
+ assertTrue(config.tags.isEmpty());
+ assertFalse(config.disables);
+ }
+
+ @Test
+ void serverWideTags_malformedFileNeverThrowsAndReportingStillWorks(@TempDir Path plugins) throws Exception {
+ // Arrange: garbage of every kind.
+ String[] contents = {
+ "tags",
+ "tags:\n :\n ::::\n \"\n '\n \\\n\t\tci:\t\"true\n",
+ "tags: {ci: true}\n",
+ "tags:\n- ci\n- \"true\"\n",
+ "\u0000\u0001tags:\n \u0000: \u0001\n",
+ };
+ for (String content : contents) {
+ received.clear();
+ arrived = new CountDownLatch(1);
+ writeServerWideConfig(plugins, content);
+ String body = assertDoesNotThrow(() -> reportedBody(plugins, "startup", null), content);
+ assertEquals("{\"application\":\"MyPlugin\",\"name\":\"startup\",\"tags\":{\"version\":\"1.2.3\"}}", body, content);
+ }
+
+ // Bytes that are not UTF-8 at all.
+ received.clear();
+ arrived = new CountDownLatch(1);
+ Path file = plugins.resolve("trace").resolve("config.yml");
+ Files.write(file, new byte[] {'t', 'a', 'g', 's', ':', '\n', ' ', ' ', 'c', 'i', ':', ' ', (byte) 0xC3, (byte) 0x28, '\n'});
+ assertEquals("{\"application\":\"MyPlugin\",\"name\":\"startup\",\"tags\":{\"version\":\"1.2.3\"}}", reportedBody(plugins, "startup", null),
+ "a file that is not UTF-8 counts as enabled with no tags");
+ }
+
+ @Test
+ void withServerWideTags_leavesTheEventAloneWhenThereAreNone() {
+ Map tags = Collections.singletonMap("name", "home");
+ assertSame(tags, TraceClient.withServerWideTags(tags, Collections.emptyMap()));
+ assertNull(TraceClient.withServerWideTags(null, Collections.emptyMap()));
+ }
+
@Test
void serverWideConfig_ioFailureIsLoggedFineAndTreatedAsEnabled(@TempDir Path scratch) throws Exception {
// Arrange
@@ -506,7 +943,7 @@ void serverWideConfig_ioFailureIsLoggedFineAndTreatedAsEnabled(@TempDir Path scr
logger.addHandler(log);
// Act
- TraceClient client = assertDoesNotThrow(() -> TraceClient.builder(baseUrl(), "MyPlugin").key("k")
+ TraceClient client = assertDoesNotThrow(() -> TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key("k")
.serverWideConfig(notADirectory.toFile()).logger(logger).build());
// Assert
@@ -520,9 +957,24 @@ void serverWideConfig_ioFailureIsLoggedFineAndTreatedAsEnabled(@TempDir Path scr
client.close();
}
- private static String lastLine(Path file) throws java.io.IOException {
- List lines = Files.readAllLines(file, StandardCharsets.UTF_8);
- return lines.get(lines.size() - 1) + "\n";
+ private static Path writeServerWideConfig(Path plugins, String content) throws java.io.IOException {
+ Path file = plugins.resolve("trace").resolve("config.yml");
+ Files.createDirectories(file.getParent());
+ Files.write(file, content.getBytes(StandardCharsets.UTF_8));
+ return file;
+ }
+
+ /** Builds a client over {@code plugins}, reports one event, and returns the body the server got. */
+ private String reportedBody(Path plugins, String name, Map tags) throws Exception {
+ TraceClient client = TraceClient.builder(baseUrl(), "MyPlugin", "1.2.3").key("k").serverWideConfig(plugins.toFile()).build();
+ client.report(name, null, tags);
+ assertTrue(arrived.await(5, TimeUnit.SECONDS), "the report should reach the server");
+ client.close();
+ return received.get(received.size() - 1).body;
+ }
+
+ private static Map tagsOf(String content) {
+ return TraceClient.parseServerWideConfig(Arrays.asList(content.split("\n", -1))).tags;
}
private static byte[] readAll(InputStream in) throws java.io.IOException {