Skip to content

Clean up version logging and name start timestamp unit correctly#286

Merged
realark merged 2 commits into
masterfrom
ark/intempt_fix
Apr 11, 2018
Merged

Clean up version logging and name start timestamp unit correctly#286
realark merged 2 commits into
masterfrom
ark/intempt_fix

Conversation

@realark
Copy link
Copy Markdown
Contributor

@realark realark commented Apr 10, 2018

  • Move java-agent version logging to a location where all version files are visible
  • Rename withStartTimestamp param to timestampMicroseconds

@palazzem From our troubleshooting session earlier today.

It turns out we actually are expecting microseconds to withStartTimestamp; the parameter name was misleading.

@realark realark added the type: bug Bug report and fix label Apr 10, 2018
@realark realark requested a review from palazzem April 10, 2018 23:57
public static void logAllVersions() {
log.info(
"dd-trace-ot - version: {}",
getVersionString(Utils.getAgentClassLoader().getResourceAsStream("dd-trace-ot.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.

I originally separated this out to highlight if someone was using an incompatible version of dd-trace-ot or the api. I don't think that this new way of doing things accomplishes that, so if this is the way we need to do things, I'm ok with just removing the duplication.

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.

Our new javaagent bootstrap process breaks the original version verification because the only jars that will be on the agent's classpath must come from the jars embedded in the dd-java-agent.jar.

I left the original version checks in there for cases where the ot and api jars are run as the main app (where they print the version string). Do you think that's worth keeping or should we just remove that logic?

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.

Yeah. Good to have the version checking ability.

@realark realark merged commit 3f26662 into master Apr 11, 2018
@realark realark deleted the ark/intempt_fix branch April 11, 2018 16:19
@tylerbenson tylerbenson added this to the 0.7.0 milestone Apr 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: bug Bug report and fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants