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

Expose mechanism to hide from stack trace #59

Merged

Conversation

cedrickcooke
Copy link
Contributor

@cedrickcooke cedrickcooke commented May 18, 2021

Exposes a mechanism to hide a class from the automatic tag stack trace on JVM. As an example of where this is useful, a library might make a custom Log object that automatically adds metadata to every call. By providing this interface, doing so will not come at the cost of ruining the tag associated with the call.

@cedrickcooke cedrickcooke requested a review from twyatt May 18, 2021 17:37
@cedrickcooke cedrickcooke marked this pull request as ready for review May 18, 2021 17:37
@cedrickcooke cedrickcooke requested review from a team and sdonn3 May 18, 2021 17:37
@codecov
Copy link

codecov bot commented May 18, 2021

Codecov Report

Merging #59 (53a5c3c) into main (b13c562) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##               main      #59   +/-   ##
=========================================
  Coverage     94.40%   94.40%           
  Complexity       52       52           
=========================================
  Files            13       13           
  Lines           125      125           
  Branches          8        8           
=========================================
  Hits            118      118           
  Misses            6        6           
  Partials          1        1           
Impacted Files Coverage Δ Complexity Δ
logging/src/commonMain/kotlin/Log.kt 100.00% <100.00%> (ø) 28.00 <1.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b13c562...53a5c3c. Read the comment docs.

Copy link
Member

@twyatt twyatt left a comment

Choose a reason for hiding this comment

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

💯

@@ -13,6 +13,7 @@ internal object StackTraceTagGenerator : TagGenerator {
private val stackTraceLineMatcher = Regex("""^\s{4}at (\w+)\..*$""")

override fun getTag(): String {
// FIXME: This should implement support for [HideFromStackTrace] instead of hard-coding a known depth
Copy link
Member

Choose a reason for hiding this comment

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

We should probably get this working before we do a release, right?
If so, it is likely deserving of a ticket so we can mark it with a milestone (to make sure we don't miss it).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now this can't be fixed, because the JS stack trace is 1) slightly different for different browsers, and 2) does not provide full package names on any of them (are packages even a concept in JS?). Regardless, the net effect is that we need support from the Kotlin compiler.

After this is merged I'll file an issue so we can at least track it. We can also discuss workarounds in that as a thread. Either way, I don't think this is a release blocker.

@cedrickcooke cedrickcooke merged commit 0859e36 into main May 18, 2021
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