-
Notifications
You must be signed in to change notification settings - Fork 105
AGTMETRICS-212 Cleanups and refactorings #269
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?
Conversation
Allows users to query the port number when it is assigned by the OS.
Remove telemetryClient, as its distinction from the other client instance was erased by time. Parameterize the test to uniformly test containerID support. Remove the dedicated containerID test. Use a single pair of Begin/After hooks, setting up new client and server for each test. Hard-code expected values of metrics_type tag on certain metrics, so they are properly tested as well as the metrics themselves.
This bit does not affect test execution
Store containerID in the main client instance and access it from the nested classes (same as the metric prefix). Remove all plumbing for containerID from the processors and message classes. Move TelemetryMessage into the client class as well so it can access the containerID the same way as other message types. Remove the Telemetry.Builder class now that the constructor only has one parameter.
It is in the wrong place, explaining the value no longer in use.
Keep lines shorter and under the checkstyle's limit.
Finding the right parameter in a list of 22 is tricky, in particular since parameter names and builder fields do not all have the same names.
Move documentation from client's private constructor into the builder, where it is visible to the user. Edit a bit for clarity.
Co-authored-by: datadog-datadog-prod-us1[bot] <88084959+datadog-datadog-prod-us1[bot]@users.noreply.github.com>
Co-authored-by: datadog-datadog-prod-us1[bot] <88084959+datadog-datadog-prod-us1[bot]@users.noreply.github.com>
Co-authored-by: datadog-datadog-prod-us1[bot] <88084959+datadog-datadog-prod-us1[bot]@users.noreply.github.com>
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.
Just a couple of minor nits on the comments.
@@ -60,7 +60,7 @@ protected Message(String aspect, Message.Type type, String[] tags) { | |||
* @return boolean indicating whether the message was partially written to the builder. | |||
* If true, the method will be called again with the same arguments to continue writing. | |||
*/ | |||
abstract boolean writeTo(StringBuilder builder, int capacity, String containerID); | |||
abstract boolean writeTo(StringBuilder builder, int capacity); |
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.
containerID
needs to be removed from the comment.
* Origin detection can be disabled by configuring the environment variabe DD_ORIGIN_DETECTION_ENABLED=false | ||
* The client tries to read the container ID by parsing the file /proc/self/cgroup. | ||
* This is not supported on Windows. | ||
* The client prioritizes the value passed via or entityID or DD_ENTITY_ID (if set) over the container ID. |
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.
I'm not sure if this is strictly true. From what I can see, if both entityID and container ID are set both values will be sent and it is the Agent rather than the client that will prioritise entityID.
No functional changes. Some pushing code around to make future features easier to implement, whitespace cleanup, move some apidocs to a place where they will be actually picked up by javadoc.