Skip to content

ref: Remove transaction_start from MSTeams integration#64167

Merged
szokeasaurusrex merged 3 commits intomasterfrom
szokeasaurusrex/msteams-tx-start
Jan 30, 2024
Merged

ref: Remove transaction_start from MSTeams integration#64167
szokeasaurusrex merged 3 commits intomasterfrom
szokeasaurusrex/msteams-tx-start

Conversation

@szokeasaurusrex
Copy link
Copy Markdown
Member

@szokeasaurusrex szokeasaurusrex commented Jan 30, 2024

Using transaction_start in the MSTeams integration routes is unnecessary and causes undefined behavior. All the routes in this integration are already automatically instrumented by the Django integration, and the auto-instrumented transactions can be viewed here.

ref #63951

Using `transaction_start` in the MSTeams integration routes is unnecessary and causes undefined behavior. All the routes in this integration are already automatically instrumented by the Django integration, and the auto-instrumented transactions can be viewed [here](https://sentry.sentry.io/performance/?isDefaultQuery=false&metricSearchSetting=metricsOnly&project=1&query=%2Fextensions%2Fmsteams%2F%2A&statsPeriod=24h).

ref #63951
@szokeasaurusrex szokeasaurusrex requested a review from a team as a code owner January 30, 2024 14:46
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jan 30, 2024
@szokeasaurusrex szokeasaurusrex enabled auto-merge (squash) January 30, 2024 14:47
@codecov
Copy link
Copy Markdown

codecov bot commented Jan 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (3b8a9f4) 77.67% compared to head (8016eed) 81.25%.

❗ Current head 8016eed differs from pull request most recent head 5a4786d. Consider uploading reports for the commit 5a4786d to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #64167      +/-   ##
==========================================
+ Coverage   77.67%   81.25%   +3.57%     
==========================================
  Files        5233     5234       +1     
  Lines      230442   230447       +5     
  Branches    45210    45205       -5     
==========================================
+ Hits       179007   187250    +8243     
+ Misses      45307    37356    -7951     
+ Partials     6128     5841     -287     
Files Coverage Δ
src/sentry/integrations/msteams/link_identity.py 94.11% <ø> (+35.78%) ⬆️
src/sentry/integrations/msteams/unlink_identity.py 93.93% <ø> (-0.35%) ⬇️
src/sentry/integrations/msteams/webhook.py 89.06% <ø> (+23.25%) ⬆️

... and 601 files with indirect coverage changes

@szokeasaurusrex szokeasaurusrex changed the title Remove transaction_start from MSTeams integration ref: Remove transaction_start from MSTeams integration Jan 30, 2024
@szokeasaurusrex szokeasaurusrex enabled auto-merge (squash) January 30, 2024 15:27
Copy link
Copy Markdown
Contributor

@asottile-sentry asottile-sentry left a comment

Choose a reason for hiding this comment

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

@szokeasaurusrex szokeasaurusrex merged commit f94aee0 into master Jan 30, 2024
@szokeasaurusrex szokeasaurusrex deleted the szokeasaurusrex/msteams-tx-start branch January 30, 2024 16:25
snigdhas pushed a commit that referenced this pull request Jan 30, 2024
Using `transaction_start` in the MSTeams integration routes is
unnecessary and causes undefined behavior. All the routes in this
integration are already automatically instrumented by the Django
integration, and the auto-instrumented transactions can be viewed
[here](https://sentry.sentry.io/performance/?isDefaultQuery=false&metricSearchSetting=metricsOnly&project=1&query=%2Fextensions%2Fmsteams%2F%2A&statsPeriod=24h).

ref #63951
@github-actions github-actions bot locked and limited conversation to collaborators Feb 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants