Adding warning to log/log_event calls#255
Conversation
codeboten
requested review from
Oberon00,
a-feld,
c24t,
carlosalberto,
lzchen,
reyang and
toumorokoshi
as code owners
c24t
left a comment
c24t
left a comment
There was a problem hiding this comment.
LGTM with only minor comments, welcome to the project!
| self.assertEqual( | ||
| span.unwrap().events[0].attributes["event"], "foo" | ||
| ) | ||
| self.assertEqual( | ||
| span.unwrap().events[0].attributes["payload"], "bar" | ||
| ) | ||
| self.assertIsNotNone(span.unwrap().events[0].timestamp) |
There was a problem hiding this comment.
Any reason for these to be inside the context manager?
There was a problem hiding this comment.
There is not. I just copied the test_log_kv format. Will update
| return self | ||
|
|
||
| def log(self, **kwargs): | ||
| logger.warning("Calling deprecated method log") |
There was a problem hiding this comment.
Good reminder that we should consider deprecated for this kind of thing.
There was a problem hiding this comment.
Can do, you want me to add Deprecated as an install dependency of the shim?
There was a problem hiding this comment.
Sorry, I didn't mean to suggest any action in this PR, just a reminder that we might want to do this in the future.
We talked about adding the library earlier but didn't yet have a good use case for it. This might be a good use case.
Tested that calling log/log_event worked without implementing them. The implementation is strictly to log a warning when those deprecated methods are called. - Use deprecated instead of logging directly Signed-off-by: Alex Boten <aboten@lightstep.com>
| self._otel_span.add_event(event_name, event_timestamp, key_values) | ||
| return self | ||
|
|
||
| @deprecated(reason="This method is deprecated in favor of log_kv") |
There was a problem hiding this comment.
(I feel we should add these to opentracing itself too ;) )
Signed-off-by: Alex Boten <aboten@lightstep.com>
|
I signed it |
3 participants
Tested that calling log/log_event worked without implementing them. The implementation is strictly to log a warning when those deprecated methods are called.
Signed-off-by: Alex Boten aboten@lightstep.com