Skip to content
This repository was archived by the owner on Aug 30, 2023. It is now read-only.

Conversation

@maciejwalkowiak
Copy link
Contributor

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

Logback integration gets now an option to set minimumEventLevel and minimumBreadcrumbLevel. Breadcrumbs are attached to current scope and sent along with following SentryEvent.

💡 Motivation and Context

Requested by @bruno-garcia.

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • All tests passing
  • No breaking changes

Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

This is good stuff!!
Left a couple of nits

}

@Test
fun `attaches breadcrumbs`() {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
fun `attaches breadcrumbs`() {
fun `attaches breadcrumbs with level higher than minimumBreadcrumbLevel`() {

Can we have some more use cases here? Like level higher than breadcrumb is captured as event but not added as breadcrumbs.
Set only level for breadcrumb, only for event, etc?

fixture = Fixture(minimumBreadcrumbLevel = Level.DEBUG, minimumEventLevel = WARN)
val utcTime = LocalDateTime.now(ZoneId.of("UTC"))

fixture.logger.debug("this should be a breadcrumb #1")
Copy link
Member

Choose a reason for hiding this comment

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

One test case could make sure this doesn't end up in a crumb because min level is Info.

Actually a test case for default would be nice, and default would be Info or higher for Breadcrumb and Error or higher for event.

@bruno-garcia bruno-garcia merged commit 82e4efe into feat/sentry-java Aug 31, 2020
@bruno-garcia bruno-garcia deleted the logback-breadcrumbs branch August 31, 2020 22:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants