-
Notifications
You must be signed in to change notification settings - Fork 386
Add logging #252
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
Add logging #252
Conversation
|
|
||
| def generate(self): | ||
| text = self.text | ||
| self.logger.debug("Genearting text: %s" % self.text) |
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.
Typo here
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.
fixed
dan1
left a comment
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 have a small typo, other than that looks good.
shakeelmohamed
left a comment
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.
Can you add a screenshot that verifies the logging works?
| count = Option(require=True, validate=validators.Integer(0)) | ||
|
|
||
| def generate(self): | ||
| self.logger.debug("Generating %d words" % self.count) |
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.
replace words with events?
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.
done
|
|
||
| def generate(self): | ||
| text = self.text | ||
| self.logger.debug("Genearting text: %s" % self.text) |
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.
typo: generating
Should we say Generating %d events with text %s?
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.
done
| propagate = 0 ; Default: 1 | ||
| level = NOTSET ; Default: WARNING | ||
| handlers = splunklib ; Default: stderr | ||
| propagate = 0 ; Default: 1 |
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.
should also update the logger_GenerateHelloCommand stanza
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.
done
|
@xuchen509 why do the logs contain hardcoded |
|
@shakeelmohamed i don't know where those come from, that's old logs, not related to this change. Also I can't find anywhere we hard code David's machine path. |
shakeelmohamed
left a comment
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.
@xuchen509 there is a tess looking at stdout and failing due to the new logging changes.
======================================================================
FAIL: test_sum_as_unit (searchcommands.test_searchcommands_app.TestSearchCommandsApp)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/travis/build/splunk/splunk-sdk-python/tests/searchcommands/test_searchcommands_app.py", line 230, in test_sum_as_unit
self.assertEqual('', errors)
AssertionError: u'' != '2019-03-27 20:45:12,006, Level=ERROR, Pi[203 chars]t.\n'
| expected, output, errors, exit_status = self._run_command('sum', action='execute', phase='map', protocol=1) | ||
| self.assertEqual(0, exit_status, msg=six.text_type(errors)) | ||
| self.assertEqual('2019-03-27 20:45:12,006, Level=ERROR, Pi[203 chars]t.', errors) | ||
| self.assertEqual('2019-03-27 20:45:12,006, Level=ERROR, Pi[203 chars]t.\n', errors) |
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.
this shouldn't be a hardcoded timestamp, use a regex or substring match
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 reverted back to the previous one and use splunklib handler instead of stderr. I can't reproduce this error locally tho.
Add logging to custom search commands app to showcase how to do logging in custom search commands by using splunk sdk