Skip to content

Stop logging GraphQL query and adds searchterm log - #983

Merged
JPrevost merged 1 commit into
mainfrom
timx-677
Aug 20, 2026
Merged

Stop logging GraphQL query and adds searchterm log#983
JPrevost merged 1 commit into
mainfrom
timx-677

Conversation

@JPrevost

Copy link
Copy Markdown
Member

Why are these changes being introduced:

  • Our logs were inclidng the full graphql query (twice)
  • This was excessively long and taking up a lot of space in our logs

Relevant ticket(s):

How does this address that need:

  • Suppresses logging the graphql query and extracts and logs just the search term

Document any side effects to this change:

  • When we move to structured logging, we'll want to include the search term as a field and not a separate logline. That is out of scope of this work but will come soon as we move from Logz to CloudWatch

Developer

  • All new ENV is documented in README
  • All new ENV has been added to Heroku Pipeline, Staging and Prod
  • ANDI or Wave has been run in accordance to
    our guide and
    all issues introduced by these changes have been resolved or opened as new
    issues (link to those issues in the Pull Request details above)
  • Stakeholder approval has been confirmed (or is not needed)

Code Reviewer

  • The commit message is clear and follows our guidelines
    (not just this pull request message)
  • There are appropriate tests covering any new functionality
  • The documentation has been updated or is unnecessary
  • The changes have been verified
  • New dependencies are appropriate or there were no changes

Requires database migrations?

NO

Includes new or updated dependencies?

NO

Why are these changes being introduced:

* Our logs were inclidng the full graphql query (twice)
* This was excessively long and taking up a lot of space in our logs

Relevant ticket(s):

* https://mitlibraries.atlassian.net/browse/TIMX-677

How does this address that need:

* Suppresses logging the graphql query and extracts and logs just the
  search term

Document any side effects to this change:

* When we move to structured logging, we'll want to include the search
  term as a field and not a separate logline. That is out of scope of
  this work but will come soon as we move from Logz to CloudWatch
@qltysh

qltysh Bot commented Aug 20, 2026

Copy link
Copy Markdown

❌ 1 blocking issue (1 total)

Tool Category Rule Count
rubocop Lint Assignment Branch Condition size for search is too high. [<7, 20, 0> 21.19/17] 1

@mitlib
mitlib temporarily deployed to timdex-api-p-timx-677-bfpsv64b August 20, 2026 15:45 Inactive
@matt-bernhardt matt-bernhardt self-assigned this Aug 20, 2026

@matt-bernhardt matt-bernhardt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems pretty straightforward. I see the different log messages between staging and this review app, which is an improvement. I agree that we'll need to tweak this again when we do more with logging, but for now this is what's needed.

:shipit:

@JPrevost
JPrevost merged commit 98e6645 into main Aug 20, 2026
3 checks passed
@JPrevost
JPrevost deleted the timx-677 branch August 20, 2026 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants