You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "Jedore (via GitHub)" <gi...@apache.org> on 2023/02/05 15:13:19 UTC
[GitHub] [skywalking-python] Jedore opened a new pull request, #276: Add loguru support
Jedore opened a new pull request, #276:
URL: https://github.com/apache/skywalking-python/pull/276
<!--
β οΈ Please make sure to read this template first, pull requests that don't accord with this template
maybe closed without notice.
Texts surrounded by `<` and `>` are meant to be replaced by you, e.g. <framework name>, <issue number>.
Put an `x` in the `[ ]` to mark the item as CHECKED. `[x]`
-->
<!-- ==== π Remove this line WHEN AND ONLY WHEN you're fixing a bug, follow the checklist π ====
### Fix <bug description or bug issue link>
- [ ] Add a unit test to verify that the fix works.
- [ ] Explain briefly why the bug exists and how to fix it.
==== π Remove this line WHEN AND ONLY WHEN you're fixing a bug, follow the checklist π ==== -->
<!-- ==== π Remove this line WHEN AND ONLY WHEN you're improving the performance, follow the checklist π ====
### Improve the performance of <class or module or ...>
- [ ] Add a benchmark for the improvement, refer to [the existing ones](https://github.com/apache/skywalking/blob/master/apm-commons/apm-datacarrier/src/test/java/org/apache/skywalking/apm/commons/datacarrier/LinkedArrayBenchmark.java)
- [ ] The benchmark result.
```text
<Paste the benchmark results here>
```
- [ ] Links/URLs to the theory proof or discussion articles/blogs. <links/URLs here>
==== π Remove this line WHEN AND ONLY WHEN you're improving the performance, follow the checklist π ==== -->
<!-- ==== π Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist π ====
### <Feature description>
- [ ] If this is non-trivial feature, paste the links/URLs to the design doc.
- [ ] Update the documentation to include this new feature.
- [ ] Tests(including UT, IT, E2E) are added to verify the new feature.
- [ ] If it's UI related, attach the screenshots below.
- [ ] I have rebuilt the `Configuration.md` documentation by running `make doc-gen`
==== π Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist π ==== -->
### Add log reporter support for loguru
- [ ] If adding a new plugin, add a component id in [the main repo](https://github.com/apache/skywalking/blob/master/oap-server/server-starter/src/main/resources/component-libraries.yml)
- [ ] If adding a new plugin, add a logo in [the UI repo](https://github.com/apache/skywalking-booster-ui/tree/main/src/assets/img/technologies)
- [x] I have added the library to `pyproject.toml` (plugin group) by running `poetry add library --group plugins`
- [x] I have rebuilt the `Plugins.md` documentation by running `make doc-gen`
- [x] Closes: https://github.com/apache/skywalking/issues/10334
- [x] Update the [`CHANGELOG.md`](https://github.com/apache/skywalking-python/blob/master/CHANGELOG.md).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Superskyyy commented on pull request #276: Add loguru support
Posted by "Superskyyy (via GitHub)" <gi...@apache.org>.
Superskyyy commented on PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#issuecomment-1419688609
> > > > One more thing, I see that there's no use of SW_formatter meaning no support for `log_reporter_layout` right? Maybe please provide such additional information like "only built-in logging module supports this configuration blabla, lorugu doesn't support this configuration blabla"
>
> > >
>
> > >
>
> > > >
>
> > >
>
> > >
>
> > > > Same goes to `log_reporter_formatted` which is the trigger of layout.
>
> > >
>
> > >
>
> > > loguru's format and record are different from logging's and SW_formatter is for logging record. So i think just use loguru raw message.
>
> > > Provide such additional information is good. But may be it's not important now. Python Agent Log Reporter aims at logging module. loguru just use a few configuration in https://skywalking.apache.org/docs/skywalking-python/next/en/setup/configuration/#log-reporter-configurations.
>
> >
>
> > I'm just thinking a situation where user thinks some config like layout also applies to loguru so they set it but see no effect. They may get confused, so adding a comment to the config will help reduce this risk.
>
>
>
> Good, add where ?
Simply add to the corresponding comment in config.py and regenerate documentation by make
doc-gen.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] wu-sheng commented on pull request #276: Add loguru support
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#issuecomment-1418917970
+ means what is in the actual data
- means this is in your expect data, but can't find in the actual data.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] wu-sheng commented on a diff in pull request #276: Add loguru support
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#discussion_r1096721722
##########
tests/plugin/data/sw_loguru/expected.data.yml:
##########
@@ -0,0 +1,61 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+logItems:
+ - serviceName: provider
+ logSize: 2
Review Comment:
Is a very precise log size good? Would this test generate more than 2 records? Such as retry.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] kezhenxu94 commented on a diff in pull request #276: Add loguru support
Posted by "kezhenxu94 (via GitHub)" <gi...@apache.org>.
kezhenxu94 commented on code in PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#discussion_r1097326067
##########
tests/plugin/data/sw_loguru/expected.data.yml:
##########
@@ -0,0 +1,185 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+logItems:
+ - serviceName: provider
+ logSize: 4
+ logs:
+ - timestamp: gt 0
+ endpoint: '/users'
+ body:
+ type: 'text'
+ content:
+ text: 'Loguru provider error reported.'
+ traceContext:
+ traceId: not null
+ traceSegmentId: not null
+ spanId: 0
+ tags:
+ data:
+ - key: level
+ value: ERROR
+ - key: logger
+ value: loguru
+ - key: thread
+ value: not null
+ - key: exception
+ value: not null
+ layer: ''
+
+ - timestamp: gt 0
+ endpoint: '/users'
+ body:
+ type: 'text'
+ content:
+ text: not null
+ traceContext:
+ traceId: not null
+ traceSegmentId: not null
+ spanId: 0
+ tags:
+ data:
+ - key: level
+ value: ERROR
+ - key: logger
+ value: root
+ - key: thread
+ value: not null
+ - key: exception
+ value: not null
+ layer: ''
+
+ - timestamp: gt 0
+ endpoint: '/users'
+ body:
+ type: 'text'
+ content:
+ text: 'Loguru provider warning reported.'
+ traceContext:
+ traceId: not null
+ traceSegmentId: not null
+ spanId: 0
+ tags:
+ data:
+ - key: level
+ value: WARNING
+ - key: logger
+ value: loguru
+ - key: thread
+ value: not null
+ layer: ''
+
+ - timestamp: gt 0
+ endpoint: '/users'
+ body:
+ type: 'text'
+ content:
+ text: 'Logging provider info reported.'
+ traceContext:
+ traceId: not null
+ traceSegmentId: not null
+ spanId: 0
+ tags:
+ data:
+ - key: level
+ value: CRITICAL
+ - key: logger
+ value: root
+ - key: thread
+ value: not null
+ layer: ''
+
+meterItems: [ ]
+
+segmentItems:
+ - serviceName: provider
+ segmentSize: 1
+ segments:
+ - segmentId: not null
+ spans:
+ - operationName: '/users'
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Http
+ startTime: gt 0
+ endTime: gt 0
+ componentId: 7014
+ isError: false
+ spanType: Entry
+ peer: not null
+ skipAnalysis: false
+ tags:
+ - key: http.method
+ value: GET
+ - key: http.url
+ value: not null
+ - key: http.status_code
+ value: '200'
+ refs:
+ - parentEndpoint: '/users'
+ networkAddress: not null
+ refType: CrossProcess
+ parentSpanId: 1
+ parentTraceSegmentId: not null,
+ parentServiceInstance: not null
+ parentService: consumer
+ traceId: not null
+
+ - serviceName: consumer
+ segmentSize: 1
+ segments:
+ - segmentId: not null
+ spans:
+ - operationName: '/users'
+ parentSpanId: 0
+ spanId: 1
+ spanLayer: Http
+ startTime: gt 0
+ endTime: gt 0
+ componentId: 7002
+ isError: false
+ spanType: Exit
+ peer: ''
Review Comment:
This should not be empty, no?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Superskyyy commented on pull request #276: Add loguru support
Posted by "Superskyyy (via GitHub)" <gi...@apache.org>.
Superskyyy commented on PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#issuecomment-1419635897
> > One more thing, I see that there's no use of SW_formatter meaning no support for `log_reporter_layout` right? Maybe please provide such additional information like "only built-in logging module supports this configuration blabla, lorugu doesn't support this configuration blabla"
>
> >
>
> > Same goes to `log_reporter_formatted` which is the trigger of layout.
>
>
>
> loguru's format and record are different from logging's and SW_formatter is for logging record. So i think just use loguru raw message.
>
>
>
> Provide such additional information is good. But may be it's not important now. Python Agent Log Reporter aims at logging module. loguru just use a few configuration in https://skywalking.apache.org/docs/skywalking-python/next/en/setup/configuration/#log-reporter-configurations.
>
>
I'm just thinking a situation where user thinks some config like layout also applies to loguru so they set it but see no effect. They may get confused, so adding a comment to the config will help reduce this risk.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] kezhenxu94 commented on a diff in pull request #276: Add loguru support
Posted by "kezhenxu94 (via GitHub)" <gi...@apache.org>.
kezhenxu94 commented on code in PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#discussion_r1097273703
##########
tests/plugin/data/sw_loguru/services/provider.py:
##########
@@ -24,21 +25,35 @@
app = FastAPI()
+ logging_logger = logging.getLogger()
+
@app.get('/users')
async def application():
time.sleep(0.5)
try:
raise Exception('Loguru Exception Test.')
- except Exception: # noqa
+ except Exception: # noqa
logger.opt(exception=True).error('Loguru provider error reported.')
+ time.sleep(0.5)
+ logging_logger.error('Logging provider error reported.', exc_info=True)
+
+ time.sleep(0.5)
+ # this will be filtered by SW_AGENT_LOG_REPORTER_LEVEL
logger.debug('Loguru provider debug reported.')
+ time.sleep(0.5)
+
logger.warning('Loguru provider warning reported.')
+ time.sleep(0.5)
+
Review Comment:
Can we remove these `time.sleep`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Superskyyy commented on pull request #276: Add loguru support
Posted by "Superskyyy (via GitHub)" <gi...@apache.org>.
Superskyyy commented on PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#issuecomment-1419303720
I will take a look at the yamls.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Superskyyy commented on pull request #276: Add loguru support
Posted by "Superskyyy (via GitHub)" <gi...@apache.org>.
Superskyyy commented on PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#issuecomment-1419349818
One more thing, I see that there's no use of SW_formatter meaning no support for config.layout right? Maybe please provide such additional information like "only built-in logging module supports this configuration blabla, lorugu doesn't support this configuration blabla"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Jedore commented on pull request #276: Add loguru support
Posted by "Jedore (via GitHub)" <gi...@apache.org>.
Jedore commented on PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#issuecomment-1419958073
ok. i try.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Superskyyy commented on pull request #276: Add loguru support
Posted by "Superskyyy (via GitHub)" <gi...@apache.org>.
Superskyyy commented on PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#issuecomment-1418202388
You forgot to install fastapi in the docker-compose command. That's why ci failed. Other than this looks good.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Jedore commented on pull request #276: Add loguru support
Posted by "Jedore (via GitHub)" <gi...@apache.org>.
Jedore commented on PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#issuecomment-1418879021
**loguru and logging are different log systemοΌthey don't affect each other. Like in `tests/plugin/sw_loguru/services/provider.py`, loguru generates 2 logitems, logging genereate 2 logitems.**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Jedore commented on pull request #276: Add loguru support
Posted by "Jedore (via GitHub)" <gi...@apache.org>.
Jedore commented on PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#issuecomment-1419006449
It seems `not null / gt 0` .etc syntax is valid in other pluginj's expected.data.yml, but invalid in loguru's expected.data.yml. I'm confulsed and blocked. π
Can you give me some suggestions? β€οΈ
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Jedore commented on pull request #276: Add loguru support
Posted by "Jedore (via GitHub)" <gi...@apache.org>.
Jedore commented on PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#issuecomment-1418409008
> Looks good, thanks. Fix the CI and let's merge!
Thank you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Jedore commented on pull request #276: Add loguru support
Posted by "Jedore (via GitHub)" <gi...@apache.org>.
Jedore commented on PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#issuecomment-1418925169
> ```
> + means what is in the actual data
> - means this is in your expect data, but can't find in the actual data.
> ```
So actual data and expect data must be the same absoublutly? And seems `not null / gt 0` .etc syntax is invalid ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Superskyyy commented on a diff in pull request #276: Add loguru support
Posted by "Superskyyy (via GitHub)" <gi...@apache.org>.
Superskyyy commented on code in PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#discussion_r1097649867
##########
skywalking/plugins/sw_loguru.py:
##########
@@ -0,0 +1,224 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import logging
+import sys
+import traceback
+from multiprocessing import current_process
+from os.path import basename, splitext
+from threading import current_thread
+
+from skywalking import config, agent
+from skywalking.protocol.common.Common_pb2 import KeyStringValuePair
+from skywalking.protocol.logging.Logging_pb2 import LogData, LogDataBody, TraceContext, LogTags, TextLog
+from skywalking.trace.context import get_context
+from skywalking.utils.exception import IllegalStateError
+from skywalking.utils.filter import sw_filter
+
+link_vector = ['https://pypi.org/project/loguru/']
+support_matrix = {
+ 'loguru': {
+ '>=3.7': ['0.6.0']
+ }
+}
+note = """"""
+
+
+def install():
+ from loguru import logger
+ from loguru._recattrs import RecordException, RecordFile, RecordLevel, RecordProcess, RecordThread
+ from loguru._datetime import aware_now
+ from loguru._get_frame import get_frame
+ from loguru._logger import start_time, context as logger_context, Logger
+ from types import MethodType
+
+ _log = logger._log
+ log_reporter_level = logging.getLevelName(config.log_reporter_level) # type: int
+
+ def gen_record(self, level_id, static_level_no, from_decorator, options, message, args, kwargs):
+ """ Generate log record as loguru.logger._log """
+ core = self._core
+
+ if not core.handlers:
+ return
+
+ (exception, depth, record, lazy, colors, raw, capture, patcher, extra) = options
+
+ frame = get_frame(depth + 2)
+
+ try:
+ name = frame.f_globals['__name__']
+ except KeyError:
+ name = None
+
+ try:
+ if not core.enabled[name]:
+ return
+ except KeyError:
+ enabled = core.enabled
+ if name is None:
+ status = core.activation_none
+ enabled[name] = status
+ if not status:
+ return
+ else:
+ dotted_name = name + '.'
+ for dotted_module_name, status in core.activation_list:
+ if dotted_name[: len(dotted_module_name)] == dotted_module_name:
+ if status:
+ break
+ enabled[name] = False
+ return
+ enabled[name] = True
+
+ current_datetime = aware_now()
+
+ if level_id is None:
+ level_icon = ' '
+ level_no = static_level_no
+ level_name = f'Level {level_no}' # not really level name, just as loguru
+ else:
+ level_name, level_no, _, level_icon = core.levels[level_id]
+
+ if level_no < core.min_level:
+ return
+
+ code = frame.f_code
+ file_path = code.co_filename
+ file_name = basename(file_path)
+ thread = current_thread()
+ process = current_process()
+ elapsed = current_datetime - start_time
+
+ if exception:
+ if isinstance(exception, BaseException):
+ type_, value, traceback = (type(exception), exception, exception.__traceback__)
+ elif isinstance(exception, tuple):
+ type_, value, traceback = exception
+ else:
+ type_, value, traceback = sys.exc_info()
+ exception = RecordException(type_, value, traceback)
+ else:
+ exception = None
+
+ log_record = {
+ 'elapsed': elapsed,
+ 'exception': exception,
+ 'extra': {**core.extra, **logger_context.get(), **extra},
+ 'file': RecordFile(file_name, file_path),
+ 'function': code.co_name,
+ 'level': RecordLevel(level_name, level_no, level_icon),
+ 'line': frame.f_lineno,
+ 'message': str(message),
+ 'module': splitext(file_name)[0],
+ 'name': name,
+ 'process': RecordProcess(process.ident, process.name),
+ 'thread': RecordThread(thread.ident, thread.name),
+ 'time': current_datetime,
+ }
+
+ if capture and kwargs:
+ log_record['extra'].update(kwargs)
+
+ if record:
+ kwargs.update(record=log_record)
+
+ if args or kwargs:
+ log_record['message'] = message.format(*args, **kwargs)
+
+ if core.patcher:
+ core.patcher(log_record)
+
+ if patcher:
+ patcher(log_record)
+
+ return log_record
+
+ def _sw_log(self, level_id, static_level_no, from_decorator, options, message, args, kwargs):
+ _log(level_id, static_level_no, from_decorator, options, message, args, kwargs)
+ record = gen_record(self, level_id, static_level_no, from_decorator, options, message, args, kwargs)
Review Comment:
NoneType bug here, your gen_record function can return None if core handler missing (from `logger.remove()`), then it will error line 156
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Jedore commented on pull request #276: Add loguru support
Posted by "Jedore (via GitHub)" <gi...@apache.org>.
Jedore commented on PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#issuecomment-1420141191
I tested and i think the block point is `exception:value` pair of logging and consumer segments pair order.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Superskyyy commented on pull request #276: Add loguru support
Posted by "Superskyyy (via GitHub)" <gi...@apache.org>.
Superskyyy commented on PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#issuecomment-1419634143
Nope, failure persists...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] wu-sheng commented on pull request #276: Add loguru support
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#issuecomment-1418038884
In the issue discussion, there is a topic about `python native logger and loguru co-exist`. What will happen in this case?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Jedore commented on a diff in pull request #276: Add loguru support
Posted by "Jedore (via GitHub)" <gi...@apache.org>.
Jedore commented on code in PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#discussion_r1097277078
##########
tests/plugin/data/sw_loguru/services/provider.py:
##########
@@ -24,21 +25,35 @@
app = FastAPI()
+ logging_logger = logging.getLogger()
+
@app.get('/users')
async def application():
time.sleep(0.5)
try:
raise Exception('Loguru Exception Test.')
- except Exception: # noqa
+ except Exception: # noqa
logger.opt(exception=True).error('Loguru provider error reported.')
+ time.sleep(0.5)
+ logging_logger.error('Logging provider error reported.', exc_info=True)
+
+ time.sleep(0.5)
+ # this will be filtered by SW_AGENT_LOG_REPORTER_LEVEL
logger.debug('Loguru provider debug reported.')
+ time.sleep(0.5)
+
logger.warning('Loguru provider warning reported.')
+ time.sleep(0.5)
+
Review Comment:
> Can we remove these `time.sleep`?
Of course. I referred other test file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] kezhenxu94 merged pull request #276: Add loguru support
Posted by "kezhenxu94 (via GitHub)" <gi...@apache.org>.
kezhenxu94 merged PR #276:
URL: https://github.com/apache/skywalking-python/pull/276
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Superskyyy commented on pull request #276: Add loguru support
Posted by "Superskyyy (via GitHub)" <gi...@apache.org>.
Superskyyy commented on PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#issuecomment-1419704364
So from what I can see the only difference is that you removed the `exception:value` pair and changed the not null to value. That is truely weird right. The exception tag is supposed to be there but not being there.
> Seems [Jedore/skywalking-python@`test`/tests/plugin/data/sw_loguru/expected.data.yml](https://github.com/Jedore/skywalking-python/blob/test/tests/plugin/data/sw_loguru/expected.data.yml?rgh-link-date=2023-02-06T19%3A53%3A07Z) and [Jedore/skywalking-python@`test`/tests/plugin/data/sw_loguru/services/provider.py](https://github.com/Jedore/skywalking-python/blob/test/tests/plugin/data/sw_loguru/services/provider.py?rgh-link-date=2023-02-06T19%3A53%3A07Z) are valid. A simple action : [Jedore/skywalking-python/actions/runs/4107258862](https://github.com/Jedore/skywalking-python/actions/runs/4107258862) continue test it ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Jedore commented on a diff in pull request #276: Add loguru support
Posted by "Jedore (via GitHub)" <gi...@apache.org>.
Jedore commented on code in PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#discussion_r1096847000
##########
tests/plugin/data/sw_loguru/expected.data.yml:
##########
@@ -0,0 +1,61 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+logItems:
+ - serviceName: provider
+ logSize: 2
Review Comment:
> Is a very precise log size good? Would this test generate more than 2 records? Such as retry.
Got it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] wu-sheng commented on pull request #276: Add loguru support
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#issuecomment-1418884970
> And i'm not familiar with yaml. expected.data.yml takes too much time and still seems not valid. π’
What do you mean? Anything blocks you?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Jedore commented on pull request #276: Add loguru support
Posted by "Jedore (via GitHub)" <gi...@apache.org>.
Jedore commented on PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#issuecomment-1418045188
I would supply tomorrow.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Jedore commented on pull request #276: Add loguru support
Posted by "Jedore (via GitHub)" <gi...@apache.org>.
Jedore commented on PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#issuecomment-1418889966
> > And i'm not familiar with yaml. expected.data.yml takes too much time and still seems not valid. π’
>
> What do you mean? Anything blocks you?
I run the test command in my repo, but failed, as:
```text
=================================== FAILURES ===================================
____________________ TestPlugin.test_plugin[loguru==0.6.0] _____________________
self = <tests.plugin.data.sw_loguru.test_loguru.TestPlugin object at 0x7fae8eb14fd0>
docker_compose = <testcontainers.compose.DockerCompose object at 0x7fae8ea683a0>
version = 'loguru==0.6.0'
@pytest.mark.parametrize('version', get_test_vector(lib_name='loguru', support_matrix=support_matrix))
def test_plugin(self, docker_compose, version):
> self.validate()
tests/plugin/data/sw_loguru/test_loguru.py:36:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
self = <tests.plugin.data.sw_loguru.test_loguru.TestPlugin object at 0x7fae8eb14fd0>
expected_file_name = '/home/runner/work/skywalking-python/skywalking-python/tests/plugin/data/sw_loguru/expected.data.yml'
def validate(self, expected_file_name=None):
# type: (str) -> Response
if expected_file_name is None:
expected_file_name = os.path.join(dirname(inspect.getfile(self.__class__)), 'expected.data.yml')
# time.sleep(10)
with open(expected_file_name) as expected_data_file:
expected_data = os.linesep.join(expected_data_file.readlines())
response = requests.post(url='http://localhost:12800/dataValidate', data=expected_data)
if response.status_code != 200:
res = requests.get('http://localhost:12800/receiveData')
actual_data = yaml.dump(yaml.load(res.content, Loader=Loader))
differ = Differ()
diff_list = list(differ.compare(
actual_data.splitlines(keepends=True),
yaml.dump(yaml.load(expected_data, Loader=Loader)).splitlines(keepends=True)
))
print('diff list: ')
sys.stdout.writelines(diff_list)
> assert response.status_code == 200
E AssertionError
tests/plugin/base.py:62: AssertionError
---------------------------- Captured stdout setup -----------------------------
7f20775e0631356c4823d9372b09d653db0e6540: Pulling from apache/skywalking-agent-test-tool/mock-collector
Digest: sha256:8c010883c8203a34f1239c6487855c5ba5b40a4c52faf8d21ccc3b5073438[221](https://github.com/Jedore/skywalking-python/actions/runs/4102318884/jobs/7075166151#step:6:222)
Status: Downloaded newer image for ghcr.io/apache/skywalking-agent-test-tool/mock-collector:7f20775e0631356c4823d9372b09d653db0e6540
---------------------------- Captured stderr setup -----------------------------
Creating network "sw_loguru_beyond" with the default driver
Pulling collector (ghcr.io/apache/skywalking-agent-test-tool/mock-collector:7f20775e0631356c4823d9372b09d653db0e6540)...
Creating sw_loguru_collector_1 ...
Creating sw_loguru_collector_1 ... done
Creating sw_loguru_provider_1 ...
Creating sw_loguru_provider_1 ... done
Creating sw_loguru_consumer_1 ...
Creating sw_loguru_consumer_1 ... done
----------------------------- Captured stdout call -----------------------------
diff list:
logItems:
- logSize: 4
logs:
- body:
content:
text: Loguru provider error reported.
type: text
endpoint: /users
layer: ''
tags:
data:
- key: level
value: ERROR
- key: logger
value: loguru
- key: thread
- value: MainThread
+ value: not null
- key: exception
+ value: not null
+ timestamp: gt 0
- value: "Traceback (most recent call last):\n File \"/app/services/provider.py\"\
- , line 36, in application\n raise Exception('Loguru Exception Test.')\n\
- Exception: Loguru Exception Test.\n"
- timestamp: 1675678728625
traceContext:
spanId: 0
- traceId: a4f8cfd2a60711edb5500[242](https://github.com/Jedore/skywalking-python/actions/runs/4102318884/jobs/7075166151#step:6:243)ac120004
- traceSegmentId: a4f95e52a60711edb8a30242ac120003
+ traceId: not null
+ traceSegmentId: not null
- body:
content:
+ text: not null
- text: "2023-02-06 10:18:49,125 [MainThread] ERROR root - Logging provider\
- \ error reported.\nTraceback (most recent call last):\n File \"/app/services/provider.py\"\
- , line 36, in application\n raise Exception('Loguru Exception Test.')\n\
- Exception: Loguru Exception Test."
type: text
endpoint: /users
layer: ''
tags:
data:
- key: level
value: ERROR
- key: logger
value: root
- key: thread
- value: MainThread
- timestamp: 1675678729126
+ value: not null
+ - key: exception
+ value: not null
+ timestamp: gt 0
traceContext:
spanId: 0
- traceId: a4f8cfd2a60711edb5500242ac120004
- traceSegmentId: a4f95e52a60711edb8a30242ac120003
+ traceId: not null
+ traceSegmentId: not null
- body:
content:
text: Loguru provider warning reported.
type: text
endpoint: /users
layer: ''
tags:
data:
- key: level
value: WARNING
- key: logger
value: loguru
- key: thread
- value: MainThread
- timestamp: 1675678730128
+ value: not null
+ timestamp: gt 0
traceContext:
spanId: 0
- traceId: a4f8cfd2a60711edb5500242ac120004
- traceSegmentId: a4f95e52a60711edb8a30242ac120003
+ traceId: not null
+ traceSegmentId: not null
- body:
content:
+ text: Logging provider info reported.
- text: 2023-02-06 10:18:50,628 [MainThread] CRITICAL root - Logging provider
- critical reported.
type: text
endpoint: /users
layer: ''
tags:
data:
- key: level
value: CRITICAL
- key: logger
value: root
- key: thread
- value: MainThread
- timestamp: 1675678730629
+ value: not null
+ timestamp: gt 0
traceContext:
spanId: 0
- traceId: a4f8cfd2a60711edb5500242ac120004
- traceSegmentId: a4f95e52a60711edb8a30242ac120003
+ traceId: not null
+ traceSegmentId: not null
serviceName: provider
meterItems: []
segmentItems:
- segmentSize: 1
segments:
- - segmentId: a4f95e52a60711edb8a30242ac120003
+ - segmentId: not null
spans:
- componentId: 7014
- endTime: 1675678730630
+ endTime: gt 0
isError: false
operationName: /users
parentSpanId: -1
- peer: 172.18.0.4:48448
+ peer: not null
refs:
- - networkAddress: provider:9091
+ - networkAddress: not null
parentEndpoint: /users
parentService: consumer
- parentServiceInstance: a124c208a60711edb5500242ac120004
+ parentServiceInstance: not null
parentSpanId: 1
- parentTraceSegmentId: a4f8ce4ca60711edb5500242ac120004
+ parentTraceSegmentId: not null,
refType: CrossProcess
- traceId: a4f8cfd2a60711edb5500242ac120004
+ traceId: not null
skipAnalysis: false
spanId: 0
spanLayer: Http
spanType: Entry
- startTime: 16756787[281](https://github.com/Jedore/skywalking-python/actions/runs/4102318884/jobs/7075166151#step:6:282)14
+ startTime: gt 0
tags:
- key: http.method
value: GET
- key: http.url
- value: http://provider:9091/users
+ value: not null
- key: http.status_code
value: '200'
serviceName: provider
- segmentSize: 1
segments:
- - segmentId: a4f8ce4ca60711edb5500242ac120004
+ - segmentId: not null
spans:
- componentId: 7002
- endTime: 16756787[306](https://github.com/Jedore/skywalking-python/actions/runs/4102318884/jobs/7075166151#step:6:307)30
+ endTime: gt 0
isError: false
operationName: /users
parentSpanId: 0
- peer: provider:9091
+ peer: ''
skipAnalysis: false
spanId: 1
spanLayer: Http
spanType: Exit
- startTime: 1675678728110
+ startTime: gt 0
+ tags:
+ - key: http.url
+ value: not null
+ - key: http.method
+ value: GET
+ - key: http.status_code
+ value: '200'
+ - segmentId: not null
+ spans:
+ - componentId: 7014
+ endTime: gt 0
+ isError: false
+ operationName: /users
+ parentSpanId: -1
+ peer: not null
+ skipAnalysis: false
+ spanId: 0
+ spanLayer: Http
+ spanType: Entry
+ startTime: gt 0
tags:
- key: http.method
value: GET
- key: http.url
+ value: not null
- value: http://provider:9091/users
- - key: http.status_code
- value: '200'
- - componentId: 7014
- endTime: 1675678730636
- isError: false
- operationName: /users
- parentSpanId: -1
- peer: 172.18.0.1:50156
- skipAnalysis: false
- spanId: 0
- spanLayer: Http
- spanType: Entry
- startTime: 1675678728110
- tags:
- - key: http.method
- value: GET
- - key: http.url
- value: http://0.0.0.0:9090/users
- key: http.status_code
value: '200'
serviceName: consumer
--------------------------- Captured stderr teardown ---------------------------
```
So i only can commited the fixed code firstly and look for your help.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Superskyyy commented on a diff in pull request #276: Add loguru support
Posted by "Superskyyy (via GitHub)" <gi...@apache.org>.
Superskyyy commented on code in PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#discussion_r1097569020
##########
tests/plugin/data/sw_loguru/services/provider.py:
##########
@@ -24,21 +25,35 @@
app = FastAPI()
+ logging_logger = logging.getLogger()
+
@app.get('/users')
async def application():
time.sleep(0.5)
try:
raise Exception('Loguru Exception Test.')
- except Exception: # noqa
+ except Exception: # noqa
logger.opt(exception=True).error('Loguru provider error reported.')
+ time.sleep(0.5)
+ logging_logger.error('Logging provider error reported.', exc_info=True)
+
+ time.sleep(0.5)
+ # this will be filtered by SW_AGENT_LOG_REPORTER_LEVEL
logger.debug('Loguru provider debug reported.')
+ time.sleep(0.5)
+
logger.warning('Loguru provider warning reported.')
+ time.sleep(0.5)
+
Review Comment:
I think the sleep was added because of this bug https://github.com/apache/skywalking/issues/8752 that introduced flaky tests in e2e, but it shouldn't be a problem for you in plugin tests, so can be safely removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Jedore commented on pull request #276: Add loguru support
Posted by "Jedore (via GitHub)" <gi...@apache.org>.
Jedore commented on PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#issuecomment-1421898239
This is my comitting for a complete python plugin. Although met some trouble, thank you very much. π€ @wu-sheng @kezhenxu94 @Superskyyy
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Jedore commented on pull request #276: Add loguru support
Posted by "Jedore (via GitHub)" <gi...@apache.org>.
Jedore commented on PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#issuecomment-1419543473
> One more thing, I see that there's no use of SW_formatter meaning no support for `log_reporter_layout` right? Maybe please provide such additional information like "only built-in logging module supports this configuration blabla, lorugu doesn't support this configuration blabla"
>
> Same goes to `log_reporter_formatted` which is the trigger of layout.
loguru's format and record are different from logging's and SW_formatter is for logging record. So i think just use loguru raw message.
Provide such additional information is good. But may be it's not important now. Python Agent Log Reporter aims at logging module. loguru just use a few configuration in https://skywalking.apache.org/docs/skywalking-python/next/en/setup/configuration/#log-reporter-configurations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Jedore commented on a diff in pull request #276: Add loguru support
Posted by "Jedore (via GitHub)" <gi...@apache.org>.
Jedore commented on code in PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#discussion_r1097703966
##########
skywalking/plugins/sw_loguru.py:
##########
@@ -0,0 +1,224 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+import logging
+import sys
+import traceback
+from multiprocessing import current_process
+from os.path import basename, splitext
+from threading import current_thread
+
+from skywalking import config, agent
+from skywalking.protocol.common.Common_pb2 import KeyStringValuePair
+from skywalking.protocol.logging.Logging_pb2 import LogData, LogDataBody, TraceContext, LogTags, TextLog
+from skywalking.trace.context import get_context
+from skywalking.utils.exception import IllegalStateError
+from skywalking.utils.filter import sw_filter
+
+link_vector = ['https://pypi.org/project/loguru/']
+support_matrix = {
+ 'loguru': {
+ '>=3.7': ['0.6.0']
+ }
+}
+note = """"""
+
+
+def install():
+ from loguru import logger
+ from loguru._recattrs import RecordException, RecordFile, RecordLevel, RecordProcess, RecordThread
+ from loguru._datetime import aware_now
+ from loguru._get_frame import get_frame
+ from loguru._logger import start_time, context as logger_context, Logger
+ from types import MethodType
+
+ _log = logger._log
+ log_reporter_level = logging.getLevelName(config.log_reporter_level) # type: int
+
+ def gen_record(self, level_id, static_level_no, from_decorator, options, message, args, kwargs):
+ """ Generate log record as loguru.logger._log """
+ core = self._core
+
+ if not core.handlers:
+ return
+
+ (exception, depth, record, lazy, colors, raw, capture, patcher, extra) = options
+
+ frame = get_frame(depth + 2)
+
+ try:
+ name = frame.f_globals['__name__']
+ except KeyError:
+ name = None
+
+ try:
+ if not core.enabled[name]:
+ return
+ except KeyError:
+ enabled = core.enabled
+ if name is None:
+ status = core.activation_none
+ enabled[name] = status
+ if not status:
+ return
+ else:
+ dotted_name = name + '.'
+ for dotted_module_name, status in core.activation_list:
+ if dotted_name[: len(dotted_module_name)] == dotted_module_name:
+ if status:
+ break
+ enabled[name] = False
+ return
+ enabled[name] = True
+
+ current_datetime = aware_now()
+
+ if level_id is None:
+ level_icon = ' '
+ level_no = static_level_no
+ level_name = f'Level {level_no}' # not really level name, just as loguru
+ else:
+ level_name, level_no, _, level_icon = core.levels[level_id]
+
+ if level_no < core.min_level:
+ return
+
+ code = frame.f_code
+ file_path = code.co_filename
+ file_name = basename(file_path)
+ thread = current_thread()
+ process = current_process()
+ elapsed = current_datetime - start_time
+
+ if exception:
+ if isinstance(exception, BaseException):
+ type_, value, traceback = (type(exception), exception, exception.__traceback__)
+ elif isinstance(exception, tuple):
+ type_, value, traceback = exception
+ else:
+ type_, value, traceback = sys.exc_info()
+ exception = RecordException(type_, value, traceback)
+ else:
+ exception = None
+
+ log_record = {
+ 'elapsed': elapsed,
+ 'exception': exception,
+ 'extra': {**core.extra, **logger_context.get(), **extra},
+ 'file': RecordFile(file_name, file_path),
+ 'function': code.co_name,
+ 'level': RecordLevel(level_name, level_no, level_icon),
+ 'line': frame.f_lineno,
+ 'message': str(message),
+ 'module': splitext(file_name)[0],
+ 'name': name,
+ 'process': RecordProcess(process.ident, process.name),
+ 'thread': RecordThread(thread.ident, thread.name),
+ 'time': current_datetime,
+ }
+
+ if capture and kwargs:
+ log_record['extra'].update(kwargs)
+
+ if record:
+ kwargs.update(record=log_record)
+
+ if args or kwargs:
+ log_record['message'] = message.format(*args, **kwargs)
+
+ if core.patcher:
+ core.patcher(log_record)
+
+ if patcher:
+ patcher(log_record)
+
+ return log_record
+
+ def _sw_log(self, level_id, static_level_no, from_decorator, options, message, args, kwargs):
+ _log(level_id, static_level_no, from_decorator, options, message, args, kwargs)
+ record = gen_record(self, level_id, static_level_no, from_decorator, options, message, args, kwargs)
Review Comment:
> NoneType bug here, your gen_record function can return None if core handler missing (from `logger.remove()`), then it will error line 156
yeah, so carefully π
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] wu-sheng commented on pull request #276: Add loguru support
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#issuecomment-1419015714
> It seems `not null / gt 0` .etc syntax is valid in other plugin's expected.data.yml, but invalid in loguru's expected.data.yml. I'm confulsed and blocked. π Can you give me some suggestions? β€οΈ
@jmjoy @mrproliu Could you take a look? The log verification was added through https://github.com/apache/skywalking-agent-test-tool/pull/39
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Jedore commented on a diff in pull request #276: Add loguru support
Posted by "Jedore (via GitHub)" <gi...@apache.org>.
Jedore commented on code in PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#discussion_r1097328984
##########
tests/plugin/data/sw_loguru/expected.data.yml:
##########
@@ -0,0 +1,185 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+logItems:
+ - serviceName: provider
+ logSize: 4
+ logs:
+ - timestamp: gt 0
+ endpoint: '/users'
+ body:
+ type: 'text'
+ content:
+ text: 'Loguru provider error reported.'
+ traceContext:
+ traceId: not null
+ traceSegmentId: not null
+ spanId: 0
+ tags:
+ data:
+ - key: level
+ value: ERROR
+ - key: logger
+ value: loguru
+ - key: thread
+ value: not null
+ - key: exception
+ value: not null
+ layer: ''
+
+ - timestamp: gt 0
+ endpoint: '/users'
+ body:
+ type: 'text'
+ content:
+ text: not null
+ traceContext:
+ traceId: not null
+ traceSegmentId: not null
+ spanId: 0
+ tags:
+ data:
+ - key: level
+ value: ERROR
+ - key: logger
+ value: root
+ - key: thread
+ value: not null
+ - key: exception
+ value: not null
+ layer: ''
+
+ - timestamp: gt 0
+ endpoint: '/users'
+ body:
+ type: 'text'
+ content:
+ text: 'Loguru provider warning reported.'
+ traceContext:
+ traceId: not null
+ traceSegmentId: not null
+ spanId: 0
+ tags:
+ data:
+ - key: level
+ value: WARNING
+ - key: logger
+ value: loguru
+ - key: thread
+ value: not null
+ layer: ''
+
+ - timestamp: gt 0
+ endpoint: '/users'
+ body:
+ type: 'text'
+ content:
+ text: 'Logging provider info reported.'
+ traceContext:
+ traceId: not null
+ traceSegmentId: not null
+ spanId: 0
+ tags:
+ data:
+ - key: level
+ value: CRITICAL
+ - key: logger
+ value: root
+ - key: thread
+ value: not null
+ layer: ''
+
+meterItems: [ ]
+
+segmentItems:
+ - serviceName: provider
+ segmentSize: 1
+ segments:
+ - segmentId: not null
+ spans:
+ - operationName: '/users'
+ parentSpanId: -1
+ spanId: 0
+ spanLayer: Http
+ startTime: gt 0
+ endTime: gt 0
+ componentId: 7014
+ isError: false
+ spanType: Entry
+ peer: not null
+ skipAnalysis: false
+ tags:
+ - key: http.method
+ value: GET
+ - key: http.url
+ value: not null
+ - key: http.status_code
+ value: '200'
+ refs:
+ - parentEndpoint: '/users'
+ networkAddress: not null
+ refType: CrossProcess
+ parentSpanId: 1
+ parentTraceSegmentId: not null,
+ parentServiceInstance: not null
+ parentService: consumer
+ traceId: not null
+
+ - serviceName: consumer
+ segmentSize: 1
+ segments:
+ - segmentId: not null
+ spans:
+ - operationName: '/users'
+ parentSpanId: 0
+ spanId: 1
+ spanLayer: Http
+ startTime: gt 0
+ endTime: gt 0
+ componentId: 7002
+ isError: false
+ spanType: Exit
+ peer: ''
Review Comment:
> This should not be empty, no?
π
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Superskyyy commented on pull request #276: Add loguru support
Posted by "Superskyyy (via GitHub)" <gi...@apache.org>.
Superskyyy commented on PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#issuecomment-1419334600
Is this the cause?π
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Jedore commented on pull request #276: Add loguru support
Posted by "Jedore (via GitHub)" <gi...@apache.org>.
Jedore commented on PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#issuecomment-1419551912
>  Is this the cause?π
You are awesome! π
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Jedore commented on pull request #276: Add loguru support
Posted by "Jedore (via GitHub)" <gi...@apache.org>.
Jedore commented on PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#issuecomment-1419657882
> > > One more thing, I see that there's no use of SW_formatter meaning no support for `log_reporter_layout` right? Maybe please provide such additional information like "only built-in logging module supports this configuration blabla, lorugu doesn't support this configuration blabla"
> >
> >
> > >
> >
> >
> > > Same goes to `log_reporter_formatted` which is the trigger of layout.
> >
> >
> > loguru's format and record are different from logging's and SW_formatter is for logging record. So i think just use loguru raw message.
> > Provide such additional information is good. But may be it's not important now. Python Agent Log Reporter aims at logging module. loguru just use a few configuration in https://skywalking.apache.org/docs/skywalking-python/next/en/setup/configuration/#log-reporter-configurations.
>
> I'm just thinking a situation where user thinks some config like layout also applies to loguru so they set it but see no effect. They may get confused, so adding a comment to the config will help reduce this risk.
Good, add where ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] Jedore commented on pull request #276: Add loguru support
Posted by "Jedore (via GitHub)" <gi...@apache.org>.
Jedore commented on PR #276:
URL: https://github.com/apache/skywalking-python/pull/276#issuecomment-1419660105
Seems https://github.com/Jedore/skywalking-python/blob/test/tests/plugin/data/sw_loguru/expected.data.yml and https://github.com/Jedore/skywalking-python/blob/test/tests/plugin/data/sw_loguru/services/provider.py are valid.
A simple action : https://github.com/Jedore/skywalking-python/actions/runs/4107258862
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org