-
Notifications
You must be signed in to change notification settings - Fork 3.8k
feat: Emit git commit metric and add a commit version to sys.servers for validations #19123
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
7b455c5
11e2ea1
b69eaba
52c10aa
3282811
34ce0d2
8537b17
c9bcce0
82b8fb8
0bc9e25
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -237,6 +237,7 @@ Servers table lists all discovered servers in the cluster. | |||||
| |is_leader|BIGINT|1 if the server is currently the 'leader' (for services which have the concept of leadership), otherwise 0 if the server is not the leader, or null if the server type does not have the concept of leadership| | ||||||
| |start_time|STRING|Timestamp in ISO8601 format when the server was announced in the cluster| | ||||||
| |version|VARCHAR|Druid version running on the server| | ||||||
| |build_revision|VARCHAR|The git commit of the build that produced the server binary. Empty string when running outside a packaged binary (e.g., during `mvn test`)| | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Optionally we could also wire this to the console's services view by default in web- |
||||||
| |labels|VARCHAR|Labels for the server configured using the property [`druid.labels`](../configuration/index.md)| | ||||||
| |available_processors|BIGINT|Total number of CPU processors available to the server| | ||||||
| |total_memory|BIGINT|Total memory in bytes available to the server| | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1885,7 +1885,7 @@ | |
| </execution> | ||
| </executions> | ||
| <configuration> | ||
| <dotGitDirectory>${project.basedir}/.git</dotGitDirectory> | ||
| <dotGitDirectory>${session.executionRootDirectory}/.git</dotGitDirectory> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this change needed? Or will it still work without this multi-module setting? |
||
| <dateFormatTimeZone>Etc/UTC</dateFormatTimeZone> | ||
| <skipPoms>false</skipPoms> | ||
| <format>json</format> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,16 +29,21 @@ | |
| import org.apache.druid.common.utils.SocketUtil; | ||
| import org.apache.druid.java.util.common.IAE; | ||
| import org.apache.druid.java.util.common.ISE; | ||
| import org.apache.druid.java.util.common.StringUtils; | ||
|
|
||
| import javax.annotation.Nullable; | ||
| import javax.validation.constraints.Max; | ||
| import javax.validation.constraints.NotNull; | ||
| import java.io.IOException; | ||
| import java.io.InputStream; | ||
| import java.net.InetAddress; | ||
| import java.net.URI; | ||
| import java.net.URISyntaxException; | ||
| import java.net.URL; | ||
| import java.net.UnknownHostException; | ||
| import java.util.Map; | ||
| import java.util.Objects; | ||
| import java.util.jar.Manifest; | ||
|
|
||
| /** | ||
| * | ||
|
|
@@ -92,6 +97,10 @@ public class DruidNode | |
| UNKNOWN_VERSION | ||
| ); | ||
|
|
||
| @JsonProperty | ||
| @NotNull | ||
| private final String buildRevision = StringUtils.nullToEmptyNonDruidDataString(readBuildRevisionFromManifest()); | ||
|
|
||
| @JsonProperty | ||
| private Map<String, String> labels; | ||
|
|
||
|
|
@@ -266,6 +275,33 @@ public String getVersion() | |
| return version; | ||
| } | ||
|
|
||
| public String getBuildRevision() | ||
| { | ||
| return buildRevision; | ||
| } | ||
|
|
||
| /** | ||
| * Reads the {@code Build-Revision} attribute from the MANIFEST.MF of the JAR containing this class. | ||
| * Returns null when running outside a packaged JAR (e.g., during {@code mvn test}). | ||
| */ | ||
| private static String readBuildRevisionFromManifest() | ||
| { | ||
| try { | ||
| URL classUrl = DruidNode.class.getResource(DruidNode.class.getSimpleName() + ".class"); | ||
| if (classUrl != null && "jar".equals(classUrl.getProtocol())) { | ||
| String classPath = classUrl.toString(); | ||
| String manifestPath = classPath.substring(0, classPath.lastIndexOf('!') + 1) + "/META-INF/MANIFEST.MF"; | ||
| try (InputStream is = new URL(manifestPath).openStream()) { | ||
| return new Manifest(is).getMainAttributes().getValue("Build-Revision"); | ||
| } | ||
| } | ||
| } | ||
| catch (IOException e) { | ||
| // Fall through and return null | ||
| } | ||
| return null; | ||
| } | ||
|
Comment on lines
+287
to
+303
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can create a class |
||
|
|
||
| public DruidNode withService(String service) | ||
| { | ||
| return new DruidNode(service, host, bindOnHost, plaintextPort, tlsPort, enablePlaintextPort, enableTlsPort); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,11 +45,15 @@ | |
| import org.apache.druid.java.util.metrics.TaskHolder; | ||
| import org.apache.druid.server.DruidNode; | ||
|
|
||
| import java.io.IOException; | ||
| import java.io.InputStream; | ||
| import java.lang.annotation.Annotation; | ||
| import java.net.URL; | ||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Properties; | ||
| import java.util.jar.Manifest; | ||
|
|
||
| /** | ||
| * | ||
|
|
@@ -92,6 +96,9 @@ public void configure(Binder binder) | |
| extraServiceDimensions | ||
| .addBinding("version") | ||
| .toInstance(StringUtils.nullToEmptyNonDruidDataString(version)); // Version is null during `mvn test`. | ||
| extraServiceDimensions | ||
| .addBinding("buildRevision") | ||
| .toInstance(StringUtils.nullToEmptyNonDruidDataString(getBuildRevision())); | ||
| } | ||
|
|
||
| @Provides | ||
|
|
@@ -177,4 +184,26 @@ public Emitter get() | |
| return emitter; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Reads the {@code Build-Revision} attribute from the MANIFEST.MF of the JAR that contains this class. | ||
| * Returns null when running outside a packaged JAR (e.g., during {@code mvn test}). | ||
| */ | ||
| protected String getBuildRevision() | ||
| { | ||
| try { | ||
| URL classUrl = EmitterModule.class.getResource(EmitterModule.class.getSimpleName() + ".class"); | ||
| if (classUrl != null && "jar".equals(classUrl.getProtocol())) { | ||
| String classPath = classUrl.toString(); | ||
| String manifestPath = classPath.substring(0, classPath.lastIndexOf('!') + 1) + "/META-INF/MANIFEST.MF"; | ||
| try (InputStream is = new URL(manifestPath).openStream()) { | ||
| return new Manifest(is).getMainAttributes().getValue("Build-Revision"); | ||
| } | ||
| } | ||
| } | ||
| catch (IOException e) { | ||
| // Fall through and return null | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Log the exception so it doesn't mask a legitimate issue |
||
| } | ||
| return null; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since both |
||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are user-facing docs, so I think we can exclude the note about
mvn testthat's already captured in the javadocs for devs: