[auto-discovery][jmx] send terminator after configs + fix execption handling #3426
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hush-hush
approved these changes
Jul 11, 2017
olivielpeau
reviewed
Jul 11, 2017
agent.py
Outdated
| except Exception as e: | ||
| log.exception("unable to submit YAML via pipe: %s", e) | ||
| else: | ||
| log.info("JMX SD Config via named pipe %s successfully.", name) |
There was a problem hiding this comment.
logging name here doesn't really make sense :)
Maybe you could log all the names in this entry?
[auto-discovery] typo in terminator. [auto-discovery] log names of configs piped.
hush-hush
approved these changes
Jul 11, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
We need to submit the config terminator to acknowledge we've piped everything out. Done in it's own
write()to ensure atomicity of the write. The jmxfetch will continue to greedilyread()until the configuration terminator is received.Motivation
Feedback in the sister PR: DataDog/jmxfetch#145
Additional Notes
The local FIFO nature of the named pipes should make this process safe, a
write()should not be lost and the configuration on the other end should be received in full. Additionally, the delivery of the config terminator - which marks the end of the "piping" procedure, should be guaranteed so there should be no real possibility of having the jmxfetch side fall in a busy-loop waiting for a terminator that will never be delivered.