Skip to content

feat(search-issues): Make message column available for querying #4391

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

Merged
merged 1 commit into from
Feb 26, 2024

Conversation

wedamija
Copy link
Member

This allows the message column on search issues to be queried, and stops using issue_title. We won't merge this until we've dual written data for a week or two.

Relies on #4385, #4387 Related to getsentry/sentry#50345

@wedamija wedamija requested a review from a team as a code owner June 21, 2023 00:46
@wedamija wedamija force-pushed the danf/search-issues-message-processor branch from 83d43dc to 17770e2 Compare June 21, 2023 20:43
Base automatically changed from danf/search-issues-message-processor to master June 22, 2023 17:01
@wedamija wedamija force-pushed the danf/search-issues-message-search branch from 8c59674 to 51698b5 Compare June 22, 2023 21:16
@getsentry getsentry deleted a comment from github-actions bot Jun 22, 2023
@wedamija
Copy link
Member Author

We started dual writing today, can merge in a couple weeks

@codecov
Copy link

codecov bot commented Jun 22, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.93%. Comparing base (9b40b77) to head (58791f3).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4391   +/-   ##
=======================================
  Coverage   89.93%   89.93%           
=======================================
  Files         897      897           
  Lines       43538    43552   +14     
  Branches      288      288           
=======================================
+ Hits        39155    39168   +13     
- Misses       4341     4342    +1     
  Partials       42       42           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lynnagara
Copy link
Member

lynnagara commented Jun 23, 2023

I think it's possible to add a query processor that falls back to the other column. So you can switch over immediately and keep queries simpler.

@wedamija
Copy link
Member Author

I think it's possible to add a query processor that falls back to the other column. So you can switch over immediately and keep queries simpler.

Oh like a coalesce or something along those lines?

@lynnagara
Copy link
Member

lynnagara commented Jun 23, 2023

yeah, something like that. Pretty sure you can use the ColumnToFunction mapper to map the messages column to something like coalesce(nullif(message, ""), "search_title") I'm assuming here that it's ok to fall back on the search title if message is empty string. @evanh / search team might have more opinions on it though.

@evanh
Copy link
Member

evanh commented Jun 26, 2023

Pretty sure you can use the ColumnToFunction mapper to map the messages column to something like coalesce(nullif(message, ""), "search_title")

That isn't currently supported in the YAML right now, so you'd need to write a custom query processor and add it to the config that way. If that's something you want to do we can help with that. As long as it gets removed when the cutover is complete.

@getsantry getsantry bot added the Stale label Oct 18, 2023
This allows the message column on search issues to be queried, and stops using `issue_title`. We
won't merge this until we've dual written data for a week or two.

Relies on #4385, #4387
Related to getsentry/sentry#50345
@wedamija wedamija force-pushed the danf/search-issues-message-search branch from 51698b5 to 58791f3 Compare February 23, 2024 22:20
@wedamija wedamija merged commit 4f3deba into master Feb 26, 2024
@wedamija wedamija deleted the danf/search-issues-message-search branch February 26, 2024 23:49
JoshFerge added a commit to getsentry/sentry that referenced this pull request Feb 27, 2024
we noticed prod CI start failing here:

https://github.com/getsentry/sentry/actions/runs/8058942260/job/22012542947?pr=65859

we xfailed the test in order to unblock master.
#65858

it was determined that this test failure was due to an update made to
snuba:
getsentry/snuba#4391

where we changed how the message parameter mapped to a clickhouse search
column.

We change the operator to contains instead of equals, as now the
"message" column is filled with additional things. the test should work
correctly now.


We should do a post-mortem as to why the snuba CI did not catch this
test failure. For posterity, developers looking to replicate issues in
CI should make sure their local snuba image is up to date with what's in
prod (sentry devservices down snuba && sentry devservices up snuba)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants