Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-21043

Qserv log diet: use named context for query ID

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: Qserv
    • Labels:

      Description

      Bunch of logging messages from qserv uses explicit "QI=12345" (or "QI=12345:123") to identify query ID. It would be better to replace it with a logging context if possible.

        Attachments

          Activity

          Hide
          salnikov Andy Salnikov added a comment - - edited

          OK, playing a bit with regular expressions (it will be a big messy one) I think I found a way to keep John happy. John Gates, what do you think about this format:

          [2019-09-03T11:05:26.324-0700] [LWP:19672,CZID:1,,QID:2913] DEBUG qdisp.JobQuery (core/modules/qdisp/JobQuery.cc:57) - JobQuery QI=2913:0; desc=0x7f5bbc00adb0
          [2019-09-03T11:05:26.324-0700] [LWP:19672,CZID:1,,QID:2913#12] DEBUG qdisp.Executive (core/modules/qdisp/Executive.cc:432) - Success TRACKING QI=2913:0; size=1
          

          i.e. job id is a part of QID key separated from query ID by hash sign?

          With that I thing I can extract all pieces with one regular expression which looks as trivial as this:

            format /\[(?<time>\S+T\S+)\] \[((LWP:(?<thread_id>\d+)|CZID:(?<czar_id>\d*)|TID:(?<query_id_tmp>\d*)|QID:((?<job_id>(?<query_id>\d*)#\d*)|(?<query_id>\d*))|[^ ,]*),?)+\] (?<level>\S+)\s+(?<message>.*)/ 
          

          Show
          salnikov Andy Salnikov added a comment - - edited OK, playing a bit with regular expressions (it will be a big messy one) I think I found a way to keep John happy. John Gates , what do you think about this format: [2019-09-03T11:05:26.324-0700] [LWP:19672,CZID:1,,QID:2913] DEBUG qdisp.JobQuery (core/modules/qdisp/JobQuery.cc:57) - JobQuery QI=2913:0; desc=0x7f5bbc00adb0 [2019-09-03T11:05:26.324-0700] [LWP:19672,CZID:1,,QID:2913#12] DEBUG qdisp.Executive (core/modules/qdisp/Executive.cc:432) - Success TRACKING QI=2913:0; size=1 i.e. job id is a part of QID key separated from query ID by hash sign? With that I thing I can extract all pieces with one regular expression which looks as trivial as this: format /\[(?<time>\S+T\S+)\] \[((LWP:(?<thread_id>\d+)|CZID:(?<czar_id>\d*)|TID:(?<query_id_tmp>\d*)|QID:((?<job_id>(?<query_id>\d*)#\d*)|(?<query_id>\d*))|[^ ,]*),?)+\] (?<level>\S+)\s+(?<message>.*)/
          Hide
          salnikov Andy Salnikov added a comment -

          After going back and forth with different formatting of the MDC I decided to switch to a default "%X" format, I believe this will make it more generic and extensible. And MDC will contain a single key which encodes both query ID and job ID into the same key, not exactly the same format as "QI=" before but close (to make everyone unhappy).
          So with all these changes messages will look like:

          [2019-09-05T11:42:13.575-0700] {{CZID,1}{LWP,15507}{QID,114#0}} DEBUG qdisp.QueryRequest (core/modules/qdisp/QueryRequest.cc:188) - New QueryRequest
          

          The code is ready for review, it also needed some changes in log package, and I want to split the whole thing between few people:

          (I think Christine is reviewing things faster than I'm requesting it).

          Show
          salnikov Andy Salnikov added a comment - After going back and forth with different formatting of the MDC I decided to switch to a default "%X" format, I believe this will make it more generic and extensible. And MDC will contain a single key which encodes both query ID and job ID into the same key, not exactly the same format as "QI=" before but close (to make everyone unhappy). So with all these changes messages will look like: [2019-09-05T11:42:13.575-0700] {{CZID,1}{LWP,15507}{QID,114#0}} DEBUG qdisp.QueryRequest (core/modules/qdisp/QueryRequest.cc:188) - New QueryRequest The code is ready for review, it also needed some changes in log package, and I want to split the whole thing between few people: Kian-Tat Lim - log package John Gates - qserv Christine Banek - lsp-deploy (I think Christine is reviewing things faster than I'm requesting it).
          Hide
          ktl Kian-Tat Lim added a comment -

          Log package looks OK (asked for an additional comment, and one change needs to be reverted).  Removed my name from reviewers.

          Show
          ktl Kian-Tat Lim added a comment - Log package looks OK (asked for an additional comment, and one change needs to be reverted).  Removed my name from reviewers.
          Hide
          jgates John Gates added a comment -

          It looks OK. It would be nice to avoid duplicating the monster regex but I'm that familiar enough with it to know if it's reasonably possible. Removing my name from reviewers. 

          Show
          jgates John Gates added a comment - It looks OK. It would be nice to avoid duplicating the monster regex but I'm that familiar enough with it to know if it's reasonably possible. Removing my name from reviewers. 
          Hide
          salnikov Andy Salnikov added a comment -

          Thanks for review everyone! That regex needs special care indeed, I could not find a way to define it in one place and reuse it in both configs, if it could be done it would simplify maintenance indeed. For now they are identical so it's just copy and paste.
          I'm going to close the ticket, lsp-deploy is not merged yet, Christine will merge it and deploy once qserv is deployed.

          Show
          salnikov Andy Salnikov added a comment - Thanks for review everyone! That regex needs special care indeed, I could not find a way to define it in one place and reuse it in both configs, if it could be done it would simplify maintenance indeed. For now they are identical so it's just copy and paste. I'm going to close the ticket, lsp-deploy is not merged yet, Christine will merge it and deploy once qserv is deployed.

            People

            • Assignee:
              salnikov Andy Salnikov
              Reporter:
              salnikov Andy Salnikov
              Reviewers:
              Christine Banek
              Watchers:
              Andy Salnikov, Christine Banek, Fritz Mueller, John Gates, Kian-Tat Lim
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: