Skip to content
Snippets Groups Projects

Add more extensive logging guarded by feature flag

Merged Gosia Ksionek requested to merge mk-add-logs into master

What does this MR do and why?

Add logging every log of LLM cycle. It is guarded by the feature flag scoped to user.

Some explanation: I was thinking of overriding debug method in Llm::Logger class and adding the condition there. I decided against it as it would made the logic a bit hidden and making it less readable.

In the course of the review, I created info_or_debug method that encapsulates the feature flag logic.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Before After

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

  1. In rails console enable the experiment fully
    Feature.enable(:expanded_ai_loggging)
  2. Ask some question in chat.
  3. check if entries in llm.log are logged on info level.
  4. Switch feature flag off, ask question again and compare llm.log file content.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

solves https://gitlab.com/gitlab-org/gitlab/-/issues/432688

Edited by Gosia Ksionek

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • 2 Warnings
    :warning: 44020675: Commits that change 30 or more lines across at least 3 files should describe these changes in the commit body. For more information, take a look at our Commit message guidelines.
    :warning: 85740886: Commits that change 30 or more lines across at least 3 files should describe these changes in the commit body. For more information, take a look at our Commit message guidelines.
    1 Message
    :book: CHANGELOG missing:

    If this merge request needs a changelog entry, add the Changelog trailer to the commit message you want to add to the changelog.

    If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.

    Reviewer roulette

    Changes that require review have been detected!

    Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:

    Category Reviewer Maintainer
    backend @thutterer profile link current availability (UTC+1, same timezone as author) @jarka profile link current availability (UTC+1, same timezone as author)

    Please check reviewer's status!

    • available Reviewer is available!
    • unavailable Reviewer is unavailable!

    Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.

    To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.

    Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.

    If needed, you can retry the :repeat: danger-review job that generated this comment.

    Generated by :no_entry_sign: Danger

    Edited by Ghost User
  • Gosia Ksionek resolved all threads

    resolved all threads

  • Gosia Ksionek added 1 commit

    added 1 commit

    • 43696dfc - Apply 1 suggestion(s) to 1 file(s)

    Compare with previous version

  • Allure report

    allure-report-publisher generated test report!

    e2e-test-on-gdk: :white_check_mark: test report for bea7983e

    expand test summary
    +-----------------------------------------------------------------------+
    |                            suites summary                             |
    +------------------+--------+--------+---------+-------+-------+--------+
    |                  | passed | failed | skipped | flaky | total | result |
    +------------------+--------+--------+---------+-------+-------+--------+
    | Monitor          | 8      | 0      | 0       | 0     | 8     | ✅     |
    | Create           | 50     | 0      | 7       | 0     | 57    | ✅     |
    | Plan             | 55     | 0      | 0       | 0     | 55    | ✅     |
    | Verify           | 31     | 0      | 0       | 0     | 31    | ✅     |
    | Govern           | 57     | 0      | 0       | 0     | 57    | ✅     |
    | Data Stores      | 23     | 0      | 0       | 0     | 23    | ✅     |
    | Package          | 0      | 0      | 1       | 0     | 1     | ➖     |
    | Framework sanity | 0      | 0      | 1       | 0     | 1     | ➖     |
    | Manage           | 0      | 0      | 1       | 0     | 1     | ➖     |
    +------------------+--------+--------+---------+-------+-------+--------+
    | Total            | 224    | 0      | 10      | 0     | 234   | ✅     |
    +------------------+--------+--------+---------+-------+-------+--------+

    e2e-package-and-test: :heavy_minus_sign: test report for bea7983e

    expand test summary
    +------------------------------------------------------------+
    |                       suites summary                       |
    +-------+--------+--------+---------+-------+-------+--------+
    |       | passed | failed | skipped | flaky | total | result |
    +-------+--------+--------+---------+-------+-------+--------+
    +-------+--------+--------+---------+-------+-------+--------+
    | Total | 0      | 0      | 0       | 0     | 0     | ➖     |
    +-------+--------+--------+---------+-------+-------+--------+
    Edited by Ghost User
  • @nicolasdular would you review please?

  • Gosia Ksionek requested review from @nicolasdular

    requested review from @nicolasdular

  • Nicolas Dular approved this merge request

    approved this merge request

  • Pipeline failure due to broken master

  • @jprovaznik would you mind performing maintainer review?

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading