Fix: Add TimberTag

[audkar]

📜 Description

Brings back TimberTag. Closes #1900

💡 Motivation and Context

We are relying heavily on TimberTag value. One of our dependencies is abusing Timber logging. We were using this tag to filter out these events. This is a blocker to upgrade Sentry for us.

I decided not to use reflection, since retrieving value is not enough. We would also have to write tag clearing logic via reflection.

Pros:

• This approach will not break if internal Timber's tag management logic change in the future.
• No need to write tag clearing logic

Cons:

• Timber will format message string, which we don't use (performance)

💚 How did you test it?

Unit test

📝 Checklist

• I reviewed the submitted code
• I added tests to verify the changes
• I updated the docs if needed
• No breaking changes

🔮 Next steps

[romtsn]

Thanks for the PR!

Cons:

• Timber will format message string, which we don't use (performance)

Unfortunately, that's why we've made that PR in the first place and gone away from the log method of timber. This was resulting in bad user experience, since Timber is adding the stacktrace to the message and this was duplicated on Sentry (since Sentry parses and displays the stacktrace nicely already). 👇

So, I'm unsure if we can accept this solution, @marandaneto thoughts?

[audkar]

Unfortunately, that's why we've made that PR in the first place and gone away from the log method of timber.

I think you misunderstood me. That message is not going to be sent with Sentry event. (as verification all previous tests are passing)

Previously you ware using message from this Timber method. After change you made this no-op. This PR doesn't change this behavior.

 override fun log(
        priority: Int,
        tag: String?,
        message: String,
        t: Throwable?
    ) {
        // no-op as we've overridden all the methods
    }

☝️ this message contains merged "message+stacktrace".

Only downside of this PR is that Timber still creates this variable. But it is not logged in Sentry events.

[romtsn]

Oh, indeed, I overlooked 🤦 LGTM, that's a clever solution.

[audkar]

"Changelog" CI Action is failing. Do I need to to something?

[romtsn]

Could you also add a changelog entry?

[romtsn]

"Changelog" CI Action is failing. Do I need to to something?

Yeah, add a changelog entry mentioning PR id, something like:

* Fix: bring back support for `Timber.tag` (#1974)

[codecov-commenter]

Codecov Report

Merging #1974 (c9f5f35) into main (b8b2aa4) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               main    #1974   +/-   ##
=========================================
  Coverage     75.60%   75.60%           
  Complexity     2270     2270           
=========================================
  Files           225      225           
  Lines          8072     8072           
  Branches        852      852           
=========================================
  Hits           6103     6103           
  Misses         1558     1558           
  Partials        411      411           

---

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 b8b2aa4...c9f5f35. Read the comment docs.

[marandaneto]

@romtsn We'd need to update docs, sine we mention the removal of this feature.
@audkar thanks for doing this.

[romtsn]

@marandaneto yeah, had that in mind, will open up a PR. I think a new patch release would worth it here, wdyt?