Uploaded image for project: 'Couchbase .NET client library'
  1. Couchbase .NET client library
  2. NCBC-1461

Do not log failed N1QL prepare queries

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.5.1
    • Component/s: library
    • Labels:

      Description

      When monitoring production environments, logging about specific N1QL queries that are failing would be very helpful.  Queries might be problematic due to bad statement syntax, targeting the wrong/missing bucket, missing index, inefficient index causing a timeout, etc.

      This log would only apply If the failure is related to the query itself (i.e. not just an HTTP communication error).  Logging the actual query text that failed would help with identifying bad query and locating it in code.

      The Warn log level is probably the most appropriate for this log.

       

      NOTE: I renamed the ticket from
      Log failed N1QL queries to assist with production monitoring
      to
      Do not log failed N1QL prepare queries

        Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

          Activity

          Hide
          mike.goldsmith Michael Goldsmith added a comment -

          Hi Brant Burnett

          I did discuss briefly with Matt last week and while we think this is a good idea, printing out the query statement could have security implications. We can discuss more.

          Show
          mike.goldsmith Michael Goldsmith added a comment - Hi Brant Burnett I did discuss briefly with Matt last week and while we think this is a good idea, printing out the query statement could have security implications. We can discuss more.
          Hide
          btburnett3 Brant Burnett added a comment -

          Ahhh, that's a good point Michael Goldsmith , I didn't think about the security implications.  However, I would point out that you're already doing it when queries fail to prepare.  So that might be something to look at, too.

          Though now I'm going to contradict myself.  If using prepared queries, the portion that would affect security should be in parameters, which aren't part of the query test.  So maybe that's fine.

          Show
          btburnett3 Brant Burnett added a comment - Ahhh, that's a good point Michael Goldsmith , I didn't think about the security implications.  However, I would point out that you're already doing it when queries fail to prepare.  So that might be something to look at, too. Though now I'm going to contradict myself.  If using prepared queries, the portion that would affect security should be in parameters, which aren't part of the query test.  So maybe that's fine.
          Hide
          mike.goldsmith Michael Goldsmith added a comment -

          I agree we should not be logging failed prepare statements either.

          We need a consistent and safe way of logging both failed ad-hoc and prepared queries.

          Show
          mike.goldsmith Michael Goldsmith added a comment - I agree we should not be logging failed prepare statements either. We need a consistent and safe way of logging both failed ad-hoc and prepared queries.
          Hide
          mike.goldsmith Michael Goldsmith added a comment -

          After discussing with SDK team, we feel that the SDK should not be responsible for logging this information. All the details can be logged by the consuming application if it chooses to do so between the original request and the result.

          The commit I've added removes the logging for failed prepare statement so that we are consistent across SDKs.

          Show
          mike.goldsmith Michael Goldsmith added a comment - After discussing with SDK team, we feel that the SDK should not be responsible for logging this information. All the details can be logged by the consuming application if it chooses to do so between the original request and the result. The commit I've added removes the logging for failed prepare statement so that we are consistent across SDKs.

            People

            • Assignee:
              mike.goldsmith Michael Goldsmith
              Reporter:
              btburnett3 Brant Burnett
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Gerrit Reviews

                There are no open Gerrit changes

                  PagerDuty

                  Error rendering 'com.pagerduty.jira-server-plugin:PagerDuty'. Please contact your Jira administrators.