Skip to content
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

Default to utf-8 when no encoding type found parsing dogwrap output #268

Merged
merged 1 commit into from Jun 26, 2018

Conversation

glasnt
Copy link
Contributor

@glasnt glasnt commented May 3, 2018

Observation:

When using dogwrap (via a Python 2 pip install) to run commands via cron, in cases where the submit_mode is set to all, it was expected that both stdout and stderr would appear in DataDog. This was not the case.

The event would appear, but the stdout/stderr was lost

The output would appear correctly if the dogwrap command was run in the shell, just not in cron

Cause:

In Python 2, the string from the OutputReader was being inspected for an encoding. If this command was run within a shell, an encoding was found (in this case, ANSI_X3.4-1968). However, when run from cron, no encoding was detected. Thus, attempting to execute a string decode (line.decode(None)) resulted in an error.

In Python 3, the encoding inspection defaults to UTF-8, which is a valid encoding type.

This meant the OutputReader would end up returning no values for stdout/stderr, but the return code and command were still available and sent onto DataDog for client displaying

Solution:

In cases where an encoding type is not identified, default to UTF-8

This will ensure the type is never None, and the default value is used.

An alternative solution to this PR is to not decode if no encoding type is identified, but that might cause more issues down the line.


Environment used for debugging and code testing: Ubuntu 16.04, Python 2.7.12/3.5.2

@yannmh
Copy link
Member

yannmh commented Jun 26, 2018

Thanks for the detailed description of the problem @glasnt. I believe this is related with #55.

Setting the default encoding type to UTF-8 sounds reasonable to me.

@yannmh yannmh merged commit 37af732 into DataDog:master Jun 26, 2018
@glasnt glasnt deleted the topic/py2-encoding branch June 26, 2018 23:25
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.

None yet

2 participants