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.

Sign up for GitHub

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

Jump to bottom

Allow supplying org.slf4j.Marker #414

Merged
merged 1 commit into from
Apr 6, 2024
Merged

Allow supplying org.slf4j.Marker #414

merged 1 commit into from
Apr 6, 2024

Conversation

mlewe
Copy link
Contributor

@mlewe mlewe commented Apr 1, 2024 •
edited

This fixes #360.

./gradlew spotlessCheck fails on this PR because of the ancient version of ktfmt used in the project (0.24).

oshai
oshai reviewed Apr 2, 2024
View reviewed changes
src/javaMain/kotlin/io/github/oshai/kotlinlogging/slf4j/Slf4jExtensions.kt Outdated
/** Lazy add a log message if isTraceEnabled is true */
public fun KLogger.trace(
throwable: Throwable? = null,
marker: org.slf4j.Marker?,
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe I missed something, but why those methods needs to be added?
Anyway Marker.toSlf4j() will treat it correctly when the actual call happens, isn't 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.

I've added those extension methods as a convenience, so the user doesn't have to convert their org.slf4j.Marker to io.github.oshai.kotlinlogging.Marker manually.

If you think this is not desired, I can remove them from this PR.

What essentially happens is marker.toKotlinLogging().toSlf4j() which is equivalent to marker for the call to the underlying Logger.
(And if the underlying Logger is not Slf4j, it still behaves as a normal marker using the implementation of getName() from org.slf4j.Marker.)

Copy link
Owner

Choose a reason for hiding this comment

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

got it. I think I prefer to keep the api slim, not adding those extension methods. So it's better to remove from the PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the PR accordingly.

@mlewe mlewe changed the title Allow supplying org.slf4j.Marker directly Allow supplying org.slf4j.Marker Apr 2, 2024
oshai
oshai reviewed Apr 4, 2024
View reviewed changes
src/javaMain/kotlin/io/github/oshai/kotlinlogging/slf4j/Slf4jExtensions.kt Outdated
import io.github.oshai.kotlinlogging.KotlinLogging
import io.github.oshai.kotlinlogging.Level
import io.github.oshai.kotlinlogging.Marker
import io.github.oshai.kotlinlogging.*
Copy link
Owner

Choose a reason for hiding this comment

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

please don't use star import

src/javaMain/kotlin/io/github/oshai/kotlinlogging/slf4j/internal/Slf4jMarker.kt Outdated

import io.github.oshai.kotlinlogging.Marker

@JvmInline
Copy link
Owner

Choose a reason for hiding this comment

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

I am not sure there is a real performance gain here from inline. it might even be the opposite because of boxing / unboxing. So unless there is an obvious reason I suggest to have it a regular class.

@oshai oshai merged commit eaa5747 into oshai:master Apr 6, 2024
5 checks passed
@oshai
Copy link
Owner

oshai commented Apr 6, 2024

Thanks!

renovate bot added a commit to StefanLobbenmeier/yt-dlp-compose that referenced this pull request Apr 6, 2024
@renovate
REN - Update dependency io.github.oshai:kotlin-logging-jvm to v6.0.4 (#…
440e144 
…33)

[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[io.github.oshai:kotlin-logging-jvm](https://togithub.com/oshai/kotlin-logging)
| `6.0.3` -> `6.0.4` |
[![age](https://developer.mend.io/api/mc/badges/age/maven/io.github.oshai:kotlin-logging-jvm/6.0.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/maven/io.github.oshai:kotlin-logging-jvm/6.0.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/maven/io.github.oshai:kotlin-logging-jvm/6.0.3/6.0.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/maven/io.github.oshai:kotlin-logging-jvm/6.0.3/6.0.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>oshai/kotlin-logging
(io.github.oshai:kotlin-logging-jvm)</summary>

###
[`v6.0.4`](https://togithub.com/oshai/kotlin-logging/releases/tag/6.0.4)

[Compare
Source](https://togithub.com/oshai/kotlin-logging/compare/6.0.3...6.0.4)

#### What's Changed

- Bump actions/cache from 3.3.3 to 4.0.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[oshai/kotlin-logging#395
- Fluent Logging: Allow nullable values by
[@&#8203;Zack-Freedman-Thoughtworks](https://togithub.com/Zack-Freedman-Thoughtworks)
in
[oshai/kotlin-logging#402
- Bump multiplatform from 1.9.22 to 1.9.23 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[oshai/kotlin-logging#405
- Bump org.jetbrains.dokka from 1.9.10 to 1.9.20 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[oshai/kotlin-logging#406
- Bump actions/cache from 4.0.0 to 4.0.1 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[oshai/kotlin-logging#403
- Bump com.diffplug.spotless from 6.24.0 to 6.25.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[oshai/kotlin-logging#397
- allow only markers being supplied by
[@&#8203;ewoelfel](https://togithub.com/ewoelfel) in
[oshai/kotlin-logging#408
- Upgrade ktfmt to version 0.47 by
[@&#8203;mlewe](https://togithub.com/mlewe) in
[oshai/kotlin-logging#415
- Allow supplying org.slf4j.Marker by
[@&#8203;mlewe](https://togithub.com/mlewe) in
[oshai/kotlin-logging#414

#### New Contributors

-
[@&#8203;Zack-Freedman-Thoughtworks](https://togithub.com/Zack-Freedman-Thoughtworks)
made their first contribution in
[oshai/kotlin-logging#402
- [@&#8203;ewoelfel](https://togithub.com/ewoelfel) made their first
contribution in
[oshai/kotlin-logging#408
- [@&#8203;mlewe](https://togithub.com/mlewe) made their first
contribution in
[oshai/kotlin-logging#415

**Full Changelog**:
oshai/kotlin-logging@6.0.3...6.0.4

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Never, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/StefanLobbenmeier/yt-dlp-compose).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yNjkuMiIsInVwZGF0ZWRJblZlciI6IjM3LjI2OS4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@mlewe
Copy link
Contributor Author

mlewe commented Apr 7, 2024

Thank you and thanks for the feedback :)

nikclayton pushed a commit to pachli/pachli-android that referenced this pull request Apr 10, 2024
@renovate
fix(deps): update dependency io.github.oshai:kotlin-logging-jvm to v6…
f33d122 
….0.4 (#605)

[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[io.github.oshai:kotlin-logging-jvm](https://togithub.com/oshai/kotlin-logging)
| `6.0.3` -> `6.0.4` |
[![age](https://developer.mend.io/api/mc/badges/age/maven/io.github.oshai:kotlin-logging-jvm/6.0.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/maven/io.github.oshai:kotlin-logging-jvm/6.0.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/maven/io.github.oshai:kotlin-logging-jvm/6.0.3/6.0.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/maven/io.github.oshai:kotlin-logging-jvm/6.0.3/6.0.4?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>oshai/kotlin-logging
(io.github.oshai:kotlin-logging-jvm)</summary>

###
[`v6.0.4`](https://togithub.com/oshai/kotlin-logging/releases/tag/6.0.4)

[Compare
Source](https://togithub.com/oshai/kotlin-logging/compare/6.0.3...6.0.4)

#### What's Changed

- Bump actions/cache from 3.3.3 to 4.0.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[oshai/kotlin-logging#395
- Fluent Logging: Allow nullable values by
[@&#8203;Zack-Freedman-Thoughtworks](https://togithub.com/Zack-Freedman-Thoughtworks)
in
[oshai/kotlin-logging#402
- Bump multiplatform from 1.9.22 to 1.9.23 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[oshai/kotlin-logging#405
- Bump org.jetbrains.dokka from 1.9.10 to 1.9.20 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[oshai/kotlin-logging#406
- Bump actions/cache from 4.0.0 to 4.0.1 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[oshai/kotlin-logging#403
- Bump com.diffplug.spotless from 6.24.0 to 6.25.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[oshai/kotlin-logging#397
- allow only markers being supplied by
[@&#8203;ewoelfel](https://togithub.com/ewoelfel) in
[oshai/kotlin-logging#408
- Upgrade ktfmt to version 0.47 by
[@&#8203;mlewe](https://togithub.com/mlewe) in
[oshai/kotlin-logging#415
- Allow supplying org.slf4j.Marker by
[@&#8203;mlewe](https://togithub.com/mlewe) in
[oshai/kotlin-logging#414

#### New Contributors

-
[@&#8203;Zack-Freedman-Thoughtworks](https://togithub.com/Zack-Freedman-Thoughtworks)
made their first contribution in
[oshai/kotlin-logging#402
- [@&#8203;ewoelfel](https://togithub.com/ewoelfel) made their first
contribution in
[oshai/kotlin-logging#408
- [@&#8203;mlewe](https://togithub.com/mlewe) made their first
contribution in
[oshai/kotlin-logging#415

**Full Changelog**:
oshai/kotlin-logging@6.0.3...6.0.4

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/pachli/pachli-android).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yNjkuMiIsInVwZGF0ZWRJblZlciI6IjM3LjI2OS4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oshai oshai oshai left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Problem using LogstashMarker
2 participants
@mlewe @oshai