Skip to content

Reports JAVA_HOME (JDK/JRE path) as the task name#5861

Open
hgeraldino wants to merge 1 commit intobloomberg:mainfrom
hgeraldino:task
Open

Reports JAVA_HOME (JDK/JRE path) as the task name#5861
hgeraldino wants to merge 1 commit intobloomberg:mainfrom
hgeraldino:task

Conversation

@hgeraldino
Copy link
Copy Markdown
Contributor

@hgeraldino hgeraldino commented Apr 10, 2026

  • What is the type of the change (bug fix, feature, documentation and etc.) ?
    feature

  • What are the current behavior and expected behavior, if this is a bugfix ?
    Other interpreted languages (like Python) report their runtimes. This PR brings JVM applications on par with applications written in other languages (both interpreted languages and compiled ones)

  • What are the steps required to reproduce the bug, if this is a bugfix ?

  • What is the current behavior and new behavior, if this is a feature change or enhancement ?
    Currently, the comdb2_clientstats table reports the calling class as the task name.

> bash-5.2$ cdb2sql bmskconn dev "select task, stack FROM comdb2_clientstats  where task like 'org.%' LIMIT 1"
(task='org.jdbi.v3.core.statement.SqlLoggerUtil', stack='org.jdbi.v3.core.statement.SqlLoggerUtil.wrap(SqlLoggerUtil.java:32) 
org.jdbi.v3.core.statement.SqlStatement.internalExecute(SqlStatement.java:1819) 
org.jdbi.v3.core.result.ResultProducers.lambda$createResultBearing$4(ResultProducers.java:110) 
...
...

With this change, the task name will report the JDK/JRE used to launch the task, while the calling class will still be the first entry on the stack.

  • [Optional] Why is the new behavior better than the current behavior, if this is a feature change ?
    Helps determine which task/runtime is being used by the calling process.

…he caller class. Caller class is still included in the stack, so information is not lost

Signed-off-by: Hector Geraldino <hgeraldino@bloomberg.net>

@Deprecated
public static String getCallerClass() {
logger.warn("getCallerClass() is deprecated and will be removed in a future version");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need to print it every time this function is called?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Just wanted to note that this function is no longer called anywhere in this codebase.

I don't expect clients to be calling this function explicitly (it is not part of the JDBC API), this could've been a private or package-private method. If there's any client who is calling this function for any reason (can't think of one), then it is fine for them to see this warning IMO.

I don't think it's worth adding extra complexity to avoid additional logging for something like this, but if you strongly feel like we should, I'm happy to refactor it.



public static String getJavaHome() {
return System.getProperty("java.home");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good. I'd add a test here, too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I haven't figured out how to run tests, but also the value for this property will be different on each environment

Copy link
Copy Markdown

@roborivers roborivers left a comment

Choose a reason for hiding this comment

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

Cbuild submission: Error ⚠.
Regression testing: Success ✓.

The first 10 failing tests are:
logfill [db unavailable at finish] **quarantined**
sc_truncate [db unavailable at finish]
consumer_non_atomic_default_consumer_generated **quarantined**
reco-ddlk-sql [timeout] **quarantined**

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants