Skip to content

Conversation

@adinauer
Copy link
Member

@adinauer adinauer commented Aug 29, 2025

📜 Description

Only set template attribute for logging integrations if the formatted message differs from the template.

💡 Motivation and Context

Closes #4675

💚 How did you test it?

📝 Checklist

  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

Copy link
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@adinauer adinauer marked this pull request as ready for review August 29, 2025 10:15
@adinauer adinauer force-pushed the 08-29-only_set_log_template_if_message_differs_from_template branch from 582ae2d to 588f0f5 Compare August 29, 2025 10:19
@adinauer
Copy link
Member Author

did some happy path testing using our samples and those looked good (only adding the template when needed)

most notable things I didn't test yet:

  • logback encoder
  • log4j2 extra template patterns that don't use passed in args but things like ENV var, MDC etc.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 29, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 388.44 ms 454.31 ms 65.87 ms
Size 1.58 MiB 2.10 MiB 533.44 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
ee747ae 382.73 ms 435.41 ms 52.68 ms
ee747ae 415.92 ms 470.15 ms 54.23 ms
ee747ae 400.46 ms 423.61 ms 23.15 ms
ee747ae 554.98 ms 611.50 ms 56.52 ms
ee747ae 358.21 ms 389.41 ms 31.20 ms
3699cd5 423.60 ms 495.52 ms 71.92 ms
85d7417 347.21 ms 394.35 ms 47.15 ms
7314dbe 437.83 ms 505.64 ms 67.81 ms
ee747ae 357.79 ms 421.84 ms 64.05 ms
ee747ae 374.71 ms 455.18 ms 80.47 ms

App size

Revision Plain With Sentry Diff
ee747ae 1.58 MiB 2.10 MiB 530.95 KiB
ee747ae 1.58 MiB 2.10 MiB 530.95 KiB
ee747ae 1.58 MiB 2.10 MiB 530.95 KiB
ee747ae 1.58 MiB 2.10 MiB 530.95 KiB
ee747ae 1.58 MiB 2.10 MiB 530.95 KiB
3699cd5 1.58 MiB 2.10 MiB 533.45 KiB
85d7417 1.58 MiB 2.10 MiB 533.44 KiB
7314dbe 1.58 MiB 2.10 MiB 533.45 KiB
ee747ae 1.58 MiB 2.10 MiB 530.95 KiB
ee747ae 1.58 MiB 2.10 MiB 530.95 KiB

Previous results on branch: 08-29-only_set_log_template_if_message_differs_from_template

Startup times

Revision Plain With Sentry Diff
ed01fec 419.94 ms 472.60 ms 52.67 ms
3e8af17 543.45 ms 633.96 ms 90.51 ms
dc50773 382.94 ms 460.84 ms 77.90 ms
43b13a8 535.13 ms 613.52 ms 78.39 ms
880ef1a 394.84 ms 450.60 ms 55.76 ms
cf17b6c 382.60 ms 439.38 ms 56.78 ms

App size

Revision Plain With Sentry Diff
ed01fec 1.58 MiB 2.10 MiB 533.44 KiB
3e8af17 1.58 MiB 2.10 MiB 533.44 KiB
dc50773 1.58 MiB 2.10 MiB 533.44 KiB
43b13a8 1.58 MiB 2.10 MiB 533.44 KiB
880ef1a 1.58 MiB 2.10 MiB 533.44 KiB
cf17b6c 1.58 MiB 2.10 MiB 533.43 KiB

@adinauer
Copy link
Member Author

adinauer commented Aug 29, 2025

Log4j2 lookups work for log message with ${date:HH:mm:ss}:

		{
			"timestamp": 1756467195.051382,
			"trace_id": "e35309170e6f44ec8b4dd6f33b123e6f",
			"body": "Important 13:33:15 warning",
			"level": "warn",
			"severity_number": 13,
			"attributes": {
				"sentry.message.template": {
					"type": "string",
					"value": "Important {} warning"

However this is disabled by default due to log4shell

It just shows {} but I presume this is by design.

@adinauer
Copy link
Member Author

Logback encoder also seems to be working with an encoder that replaces every other char with _:

		{
			"timestamp": 1756468077.964525,
			"trace_id": "e963bb458fb5437d805af7c7dbf582b4",
			"body": "I_p_r_a_t _a_n_n_ m_s_a_e",
			"level": "warn",
			"severity_number": 13,
			"attributes": {
				"sentry.message.template": {
					"type": "string",
					"value": "Important warning message"
				},

@lcian lcian self-assigned this Aug 29, 2025
@alexander-alderman-webb
Copy link
Contributor

One more question, would it make sense to have unit tests for the change here?

Copy link
Collaborator

@lbloder lbloder left a comment

Choose a reason for hiding this comment

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

LGTM 👍 also +1 for unit tests

cursor[bot]

This comment was marked as outdated.

@lcian lcian merged commit 64295f6 into main Sep 3, 2025
45 of 46 checks passed
@lcian lcian deleted the 08-29-only_set_log_template_if_message_differs_from_template branch September 3, 2025 07:18
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.

Make sure the Java SDKs only set sentry.message.template on logs if sentry.message.parameter values are also defined

6 participants