abhishekrb19 commented on code in PR #19123:
URL: https://github.com/apache/druid/pull/19123#discussion_r2914689645
##########
server/src/main/java/org/apache/druid/server/emitter/EmitterModule.java:
##########
@@ -177,4 +192,33 @@ public Emitter get()
return emitter;
}
}
+
+ /**
+ * Loads git properties from the classpath resource git.properties.
+ * Returns null if the properties file is not found or cannot be loaded.
+ */
+ private static Map<String, String> loadGitProperties()
+ {
+ try (InputStream is =
EmitterModule.class.getClassLoader().getResourceAsStream("git.properties")) {
+ if (is != null) {
+ Properties props = new Properties();
+ props.load(is);
+
+ // Convert Properties to Map<String, String>
+ Map<String, String> gitProperties = new HashMap<>();
+ for (String key : props.stringPropertyNames()) {
+ String value = props.getProperty(key);
+ if (value != null && !value.trim().isEmpty()) {
+ gitProperties.put(key, value.trim());
+ }
+ }
+
+ return gitProperties;
Review Comment:
On the approach, I'm wondering whether using `git.properties` from the class
loader vs using `META-INF/MANIFEST.MF` (produced by the JAR build) would be
better.
Is the former coming from `git-commit-id-maven-plugin`:
https://github.com/apache/druid/blob/master/pom.xml#L1878-L1903?
The latter should be available through
https://github.com/apache/druid/blob/master/pom.xml#L1870:
```
Build-Revision: 1c2eb65...
Build-Version: 37.0.0-SNAPSHOT
```
Looks like version internally reads from the manifest as well: `String
version = getClass().getPackage().getImplementationVersion()`
But those are standard properties, so we may want a small utility to read
the Git SHA / build revision from the manifest, such as the `Build-Revision`
property. I’ll also give it some thought.
##########
server/src/main/java/org/apache/druid/server/emitter/EmitterModule.java:
##########
@@ -92,6 +94,19 @@ public void configure(Binder binder)
extraServiceDimensions
.addBinding("version")
.toInstance(StringUtils.nullToEmptyNonDruidDataString(version)); //
Version is null during `mvn test`.
+
+ // Add git commit SHA to all emitted metrics
+ Map<String, String> gitProperties = loadGitProperties();
+ if (gitProperties != null) {
+ extraServiceDimensions
+ .addBinding("gitSha")
+
.toInstance(StringUtils.nullToEmptyNonDruidDataString(gitProperties.get("git.commit.id.abbrev")));
+ } else {
+ // Fallback value when git properties are not available
+ extraServiceDimensions
+ .addBinding("gitSha")
Review Comment:
Thoughts on naming: `gitSha` or `buildRevision` or something else?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
