Skip to content
This repository has been archived by the owner on Nov 24, 2023. It is now read-only.

log: skip 1 caller in ErrorFilterContextCanceled() #392

Merged
merged 1 commit into from
Dec 3, 2019
Merged

log: skip 1 caller in ErrorFilterContextCanceled() #392

merged 1 commit into from
Dec 3, 2019

Conversation

kennytm
Copy link
Contributor

@kennytm kennytm commented Dec 2, 2019

What problem does this PR solve?

Without the AddCallerSkip(1) all logs going through ErrorFilterContextCanceled() will show the source as log.go:93.

What is changed and how it works?

Check List

Tests

  • Manual test (add detailed scripts or steps below)
    • Check the log after applying the patch

Code changes

Side effects

Related changes

  • Need to cherry-pick to the release branch

@kennytm
log: skip 1 caller in ErrorFilterContextCanceled()
b8acc1d 
so that not every log entry points to [log.go:93].
@kennytm kennytm added priority/unimportant Really minor change, requires approval from secondary reviewers status/PTAL This PR is ready for review. Add this label back after committing new changes type/bug-fix Bug fix needs-cherry-pick-release-1.0 This PR should be cherry-picked to release-1.0. Remove this label after cherry-picked to release-1.0 labels Dec 2, 2019
@kennytm
Copy link
Contributor Author

kennytm commented Dec 2, 2019

/run-all-tests tidb=release-3.0

PTAL @csuzhangxc @WangXiangUSTC

@codecov
Copy link

codecov bot commented Dec 2, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@3d31d97). Click here to learn what that means.
The diff coverage is 0%.

@@             Coverage Diff             @@
##             master       #392   +/-   ##
===========================================
  Coverage          ?   57.6181%           
===========================================
  Files             ?        163           
  Lines             ?      16441           
  Branches          ?          0           
===========================================
  Hits              ?       9473           
  Misses            ?       6046           
  Partials          ?        922

csuzhangxc
csuzhangxc reviewed Dec 3, 2019
View reviewed changes
Copy link
Member

@csuzhangxc csuzhangxc left a comment

Choose a reason for hiding this comment

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

LGTM

@csuzhangxc csuzhangxc added status/LGT1 One reviewer already commented LGTM and removed status/PTAL This PR is ready for review. Add this label back after committing new changes labels Dec 3, 2019
@csuzhangxc csuzhangxc added this to the v1.0.3 milestone Dec 3, 2019
WangXiangUSTC
WangXiangUSTC approved these changes Dec 3, 2019
View reviewed changes
Copy link
Contributor

@WangXiangUSTC WangXiangUSTC left a comment

Choose a reason for hiding this comment

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

LGTM

@WangXiangUSTC WangXiangUSTC added status/LGT2 Two reviewers already commented LGTM, ready for merge and removed status/LGT1 One reviewer already commented LGTM labels Dec 3, 2019
@kennytm kennytm merged commit 4a8423b into master Dec 3, 2019
@kennytm kennytm deleted the kennytm/add-caller-skip branch December 3, 2019 07:12
@sre-bot sre-bot mentioned this pull request Dec 3, 2019
Merged
@sre-bot
Copy link

sre-bot commented Dec 3, 2019

cherry pick to release-1.0 in PR #396

@sre-bot sre-bot added already-cherry-pick-1.0 The related PR is already cherry-picked to release-1.0. Add this label once the PR is cherry-picked and removed needs-cherry-pick-release-1.0 This PR should be cherry-picked to release-1.0. Remove this label after cherry-picked to release-1.0 labels Dec 3, 2019
@ericsyh ericsyh removed this from the v1.0.3 milestone Dec 3, 2019
lichunzhu pushed a commit to lichunzhu/dm that referenced this pull request Apr 6, 2020
@kennytm
log: skip 1 caller in ErrorFilterContextCanceled() (pingcap#392)
428ee92 
so that not every log entry points to [log.go:93].
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@csuzhangxc csuzhangxc csuzhangxc left review comments

@WangXiangUSTC WangXiangUSTC WangXiangUSTC approved these changes

Assignees
No one assigned
Labels
already-cherry-pick-1.0 The related PR is already cherry-picked to release-1.0. Add this label once the PR is cherry-picked priority/unimportant Really minor change, requires approval from secondary reviewers status/LGT2 Two reviewers already commented LGTM, ready for merge type/bug-fix Bug fix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kennytm @sre-bot @csuzhangxc @WangXiangUSTC @ericsyh