Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filter by log level #4445

Merged
merged 5 commits into from
Mar 2, 2023
Merged

Filter by log level #4445

merged 5 commits into from
Mar 2, 2023

Conversation

et
Copy link
Contributor

@et et commented Mar 1, 2023

Summary

This PR adds support to filter by the log level (internally called SeverityText).

Will follow up for adding support for trace id and other non-custom attributes.

This PR also replaces squirrel with go-sqlbuilder. There's a couple reasons for this:

  • squirrel is in maintenance mode where as go-sqlbuilder has active development.
  • squirrel lacks UNION support. We'll need this for permalinking (example). Per the previous point, it doesn't seem like squirrel will add union support even with a PR ready.
  • seems like squirrel has some design pitfalls (see FAQ).

How did you test this change?

Unit tests added for testing level filtering. Our existing tests should cover the squirrel -> go-sqlbuilder transition

Click test:

Kapture 2023-03-01 at 16 25 41

Are there any deployment considerations?

N/A

@et et linked an issue Mar 1, 2023 that may be closed by this pull request
@stoat-app
Copy link

stoat-app bot commented Mar 1, 2023

Easy and customizable dashboards for your build system. Learn more about Stoat ↗︎

Job Runtime

job runtime chart

debug

@et et marked this pull request as ready for review March 1, 2023 23:40
@et et requested review from a team, ccschmitz, Vadman97, aptlin and mayberryzane and removed request for a team March 1, 2023 23:41
@render
Copy link

render bot commented Mar 2, 2023

@et et force-pushed the 4443-filter-by-log-level branch from aca0f5e to bd3813f Compare March 2, 2023 17:58
@et et force-pushed the 4443-filter-by-log-level branch from bd3813f to 1aba3a9 Compare March 2, 2023 18:45
Copy link
Member

@Vadman97 Vadman97 left a comment

Choose a reason for hiding this comment

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

nice, this looks great!

@et et merged commit 2a5127a into main Mar 2, 2023
@et et deleted the 4443-filter-by-log-level branch March 2, 2023 21:24
et added a commit that referenced this pull request Mar 7, 2023
## Summary

<!--
Ideally, there is an attached Linear ticket that will describe the
"why".

If relevant, use this section to call out any additional information
you'd like to _highlight_ to the reviewer.
-->

For #4471, we intend to add copy buttons for each log property (to
prefill the query). To prepare for that, we should rename OTEL lingo to
a more product friendly lingo, specifically:

* SeverityText -> level (to coincide with #4445)
* Body -> message (since every other logging app calls it this)

Additionally, these should sit as top level keys like `trace_id`,
`span_id`, and `secure_session_id` (#4468)

## How did you test this change?

<!--
Frontend - Leave a screencast or a screenshot to visually describe the
changes.
-->

Verified that `level` and `message` are top level keys in the JSON view:

![Screenshot 2023-03-06 at 1 22 50
PM](https://user-images.githubusercontent.com/58678/223222210-caa8cf24-7498-4187-88b7-b6efc338a300.png)


## Are there any deployment considerations?

<!--
 Backend - Do we need to consider migrations or backfilling data?
-->

A follow up PR will come that will add support for searching
`message:value` (#4510)
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.

Filter by log level
2 participants