Add support printing TID to logs #323

CodePrometheus · 2023-10-11T14:32:14Z

If this pull request closes/resolves/fixes an existing issue, replace the issue url. Closes: [Feature] Python agent support printing TID to logs skywalking#10734
Update the CHANGELOG.md.

wu-sheng · 2023-10-11T14:39:00Z

Please update the docs accordingly.

CodePrometheus · 2023-10-11T14:49:52Z

Please update the docs accordingly.

done

wu-sheng · 2023-10-11T15:26:10Z

@Superskyyy Is there any doc about introducing how to use the logger?

Superskyyy · 2023-10-16T20:39:24Z

@Superskyyy Is there any doc about introducing how to use the logger?

Not about this particular feature, @CodePrometheus would you mind adding a small doc to this section (by adding a new subsection called something like ``printing traceID to logs".

Superskyyy · 2023-10-16T20:41:32Z

skywalking/loggings.py

 def getLogger(name=None):  # noqa
    logger = logging.getLogger(name)
    ch = logging.StreamHandler()
-    formatter = logging.Formatter('%(asctime)s %(name)s [pid:%(process)d] [%(threadName)s] [%(levelname)s] %(message)s')
+    formatter = logging.Formatter('%(asctime)s %(name)s %(tid)s [pid:%(process)d] [%(threadName)s] [%(levelname)s] %(message)s')


This feature should be optional as consistent with the Java agent (see here), because not all users want tids in their logs. To achieve this behavior in Python, please introduce a new setting to control the insertion of tids and adding the additional filter.

Superskyyy · 2023-10-16T20:55:35Z

@CodePrometheus Oh I just realized your understanding of this feature from conversation in #323 was not entirely accurate. Sorry, I should have pointed out earlier when you were confirming it. The place you put this code was wrong, skywalking/loggings.py is the dedicated place to hold logs of the agent itself, while the log where users want to see traceIDs are within their own application logs. You should move these code to the sw_logging module under skywalking/log. Then its all good, the code itself is correct.

CodePrometheus · 2023-10-22T17:37:02Z

Copy that, thanks for the reply, this is my negligence, please give me some time, I will improve this PR as soon as possible.

Superskyyy · 2023-10-22T21:41:58Z

Copy that, thanks for the reply, this is my negligence, please give me some time, I will improve this PR as soon as possible.

No problem! I will be waiting patiently, take your time. Ping me for any help you need.

wu-sheng · 2023-10-28T23:38:01Z

@Superskyyy As this is a terminal output, we don't have CI to verify this. Could you review and test locally?

wu-sheng · 2023-10-29T02:12:12Z

@CodePrometheus Please fix CI.

CodePrometheus · 2023-10-29T12:36:42Z

The latest CI exception does not seem to be caused by this PR change.
case1, case2.

wu-sheng · 2023-10-29T13:47:25Z

@Superskyyy Please recheck, and fix the main branch if needed.

Superskyyy · 2023-10-30T04:58:04Z

Yes, I have a fix coming, it is due to a upstream dependency bug in flask, I will hold the merge of this pr untill that one is fixed first.

Superskyyy · 2023-10-30T05:00:46Z

Thanks! The code LGTM, please regenerate the configuration documentation by running the checklist command below. I will test it locally tmr.

I have rebuilt the Configuration.md documentation by running make doc-gen

kezhenxu94 · 2023-10-31T03:37:05Z

skywalking/log/sw_logging.py

+        if config.agent_log_print_tid:
+            record.tid = str(context.segment.related_traces[0])


We add the tid info to the record only when agent_log_print_tid == true, but what if I set SW_AGENT_LOG_REPORTER_LAYOUT='%(tid)s %(asctime)s [%(threadName)s] %(levelname)s %(name)s - %(message)s but agent_log_print_tid == false? The implementation looks unreasonable to me, we have two ways to modify the log layout which might be conflict with each other.

@kezhenxu94 thanks for your comment, my opinion is:

If and only if agent_log_print_tid=true, the agent will enable the ability to print traceId.

When the user-defined layout has tid, but agent_log_print_tid=false, printing traceId is turned off, so I will determine whether the user's input already exists tid before changing the layout.

Or emphasize in the document that when agent_log_print_tid=true, the layout will change accordingly.

How about we make it more consistent with the Java agent, that is simply introducing the tid mechnism through the custom layout?

How about we make it more consistent with the Java agent, that is simply introducing the tid mechnism through the custom layout?

This makes sense to me

Superskyyy · 2023-11-02T20:16:16Z

CHANGELOG.md

@@ -4,6 +4,7 @@

 - Feature:
  - Users now can specify the `SW_AGENT_ASYNCIO_ENHANCEMENT` environment variable to enable the performance enhancement with asyncio (#316)
+  - Add support printing TID to logs (#323)


Suggested change

- Add support printing TID to logs (#323)

- Support printing Trace IDs (TID) to collected application logs (#323)

Superskyyy · 2023-11-02T20:17:01Z

docs/en/setup/advanced/LogReporter.md

+
+## Print trace ID in your logs
+To print out the traceId in the logs, simply add `%(tid)s` to the `agent_log_reporter_layout`.


Suggested change

To print out the traceId in the logs, simply add `%(tid)s` to the `agent_log_reporter_layout`.

To print out the trace IDs in the logs, simply add `%(tid)s` to the `agent_log_reporter_layout`.

Superskyyy · 2023-11-02T20:33:06Z

skywalking/log/sw_logging.py

@@ -75,6 +75,9 @@ def build_log_tags() -> LogTags:

        context = get_context()

+        if 'tid' in layout:


Please check for the full %(tid)s instead of this, otherwise one may surpise you with something like tidewave.

Superskyyy · 2023-11-02T20:43:23Z

Since tid is injected into the logging record, user could also utilize this to print out the tid in whatever channel they want other than only the reported logs to oap, this can be done using any formatter they use in their own application logic, this should be pointed out in the documentation. @CodePrometheus

Superskyyy

LGTM

Add support printing TID to logs

3f3f199

wu-sheng requested a review from Superskyyy October 11, 2023 14:38

wu-sheng added the feature New feature label Oct 11, 2023

Update CHANGELOG.md

0089ba0

Superskyyy reviewed Oct 16, 2023

View reviewed changes

fix support printing TID to logs

674e864

wu-sheng requested a review from Superskyyy October 28, 2023 23:37

fix support printing TID to logs

3367e3a

kezhenxu94 reviewed Oct 31, 2023

View reviewed changes

fix support printing TID to logs

f3fd6e4

Superskyyy requested changes Nov 2, 2023

View reviewed changes

CodePrometheus and others added 2 commits November 4, 2023 01:50

fix support printing TID to logs

21c1071

Merge branch 'master' into feature_tid_logs

95adcac

Superskyyy approved these changes Nov 4, 2023

View reviewed changes

wu-sheng added this to the 1.1.0 milestone Nov 4, 2023

wu-sheng merged commit bea712c into apache:master Nov 4, 2023
91 checks passed

CodePrometheus deleted the feature_tid_logs branch November 5, 2023 12:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support printing TID to logs #323

Add support printing TID to logs #323

CodePrometheus commented Oct 11, 2023 •

edited

Loading

wu-sheng commented Oct 11, 2023

CodePrometheus commented Oct 11, 2023

wu-sheng commented Oct 11, 2023

Superskyyy commented Oct 16, 2023

Superskyyy Oct 16, 2023

Superskyyy commented Oct 16, 2023 •

edited

Loading

CodePrometheus commented Oct 22, 2023

Superskyyy commented Oct 22, 2023

wu-sheng commented Oct 28, 2023

wu-sheng commented Oct 29, 2023

CodePrometheus commented Oct 29, 2023

wu-sheng commented Oct 29, 2023

Superskyyy commented Oct 30, 2023

Superskyyy commented Oct 30, 2023 •

edited

Loading

kezhenxu94 Oct 31, 2023

CodePrometheus Oct 31, 2023 •

edited

Loading

Superskyyy Oct 31, 2023

kezhenxu94 Nov 1, 2023

Superskyyy Nov 2, 2023

Superskyyy Nov 2, 2023

Superskyyy Nov 2, 2023

Superskyyy commented Nov 2, 2023

Superskyyy left a comment

		if config.agent_log_print_tid:
		record.tid = str(context.segment.related_traces[0])

	- Add support printing TID to logs (#323)
	- Support printing Trace IDs (TID) to collected application logs (#323)


		## Print trace ID in your logs
		To print out the traceId in the logs, simply add `%(tid)s` to the `agent_log_reporter_layout`.

	To print out the traceId in the logs, simply add `%(tid)s` to the `agent_log_reporter_layout`.
	To print out the trace IDs in the logs, simply add `%(tid)s` to the `agent_log_reporter_layout`.

		@@ -75,6 +75,9 @@ def build_log_tags() -> LogTags:

		context = get_context()

		if 'tid' in layout:

Add support printing TID to logs #323

Add support printing TID to logs #323

Conversation

CodePrometheus commented Oct 11, 2023 • edited Loading

wu-sheng commented Oct 11, 2023

CodePrometheus commented Oct 11, 2023

wu-sheng commented Oct 11, 2023

Superskyyy commented Oct 16, 2023

Superskyyy Oct 16, 2023

Choose a reason for hiding this comment

Superskyyy commented Oct 16, 2023 • edited Loading

CodePrometheus commented Oct 22, 2023

Superskyyy commented Oct 22, 2023

wu-sheng commented Oct 28, 2023

wu-sheng commented Oct 29, 2023

CodePrometheus commented Oct 29, 2023

wu-sheng commented Oct 29, 2023

Superskyyy commented Oct 30, 2023

Superskyyy commented Oct 30, 2023 • edited Loading

kezhenxu94 Oct 31, 2023

Choose a reason for hiding this comment

CodePrometheus Oct 31, 2023 • edited Loading

Choose a reason for hiding this comment

Superskyyy Oct 31, 2023

Choose a reason for hiding this comment

kezhenxu94 Nov 1, 2023

Choose a reason for hiding this comment

Superskyyy Nov 2, 2023

Choose a reason for hiding this comment

Superskyyy Nov 2, 2023

Choose a reason for hiding this comment

Superskyyy Nov 2, 2023

Choose a reason for hiding this comment

Superskyyy commented Nov 2, 2023

Superskyyy left a comment

Choose a reason for hiding this comment

CodePrometheus commented Oct 11, 2023 •

edited

Loading

Superskyyy commented Oct 16, 2023 •

edited

Loading

Superskyyy commented Oct 30, 2023 •

edited

Loading

CodePrometheus Oct 31, 2023 •

edited

Loading