Skip to content

Conversation

@kinyoklion
Copy link
Member

@kinyoklion kinyoklion commented Oct 3, 2025

Summary

The CustomSampler errantly checked if message and severity were of the correct type before attempting to match them. If they were not, then they matched. The simplest solutions is to not do the check. Alternatively it could do the check, and when the type is not string they do not match.

How did you test this change?

Unit tests.

Other

After this PR is non-js SDKs should have their JSON files updated. I did not update in this PR because I didn't want to trigger a release of those SDKs.


Note

Ensure log rules requiring message/severity only match when the record has a string message/severity; add scenarios and test helpers across SDKs.

  • Sampling logic:
    • Go (go/internal/otel/custom_sampler.go): In matchesLogConfig, require record.Body() to be KindString for message matching; return false otherwise.
    • JS/TS (Node, Shared, Highlight Run): Simplify matchesLogConfig by removing permissive typeof gates; Shared explicitly rejects non-string record.body for message matching.
  • Tests:
    • Add log scenarios that verify configs with both message and severityText do not match when logs are empty, only have message, or only have severity (in Go/Node/Shared/Highlight Run JSONs).
    • Introduce helper to construct ReadableLogRecord from scenario input in Node/Shared tests and update usages.

Written by Cursor Bugbot for commit 67c9367. This will update automatically on new commits. Configure here.

@kinyoklion
Copy link
Member Author

bugbot review

cursor[bot]

This comment was marked as outdated.

@kinyoklion kinyoklion force-pushed the rlamb/fix-sampling-over-matching-for-empty-records branch from 00fcf18 to ec85e60 Compare October 3, 2025 22:40
@kinyoklion kinyoklion marked this pull request as ready for review October 3, 2025 22:59
@kinyoklion kinyoklion requested a review from a team as a code owner October 3, 2025 22:59
…github.com:launchdarkly/observability-sdk into rlamb/fix-sampling-over-matching-for-empty-records
@kinyoklion kinyoklion merged commit 500a6bf into main Oct 6, 2025
22 checks passed
@kinyoklion kinyoklion deleted the rlamb/fix-sampling-over-matching-for-empty-records branch October 6, 2025 17:20
kinyoklion pushed a commit that referenced this pull request Oct 6, 2025
🤖 I have created a release *beep* *boop*
---


<details><summary>launchdarkly-observability-android: 0.10.0</summary>

##
[0.10.0](launchdarkly-observability-android-0.9.0...launchdarkly-observability-android-0.10.0)
(2025-10-06)


### Features

* Cache OpenTelemetry metric instruments
([#256](#256))
([db67867](db67867))
</details>

<details><summary>go: 0.3.1</summary>

##
[0.3.1](go/v0.3.0...go/v0.3.1)
(2025-10-06)


### Bug Fixes

* Fix an issue where logging configuration could over-match for log
records without a message or severity.
([#261](#261))
([500a6bf](500a6bf))
</details>

<details><summary>observability: 0.4.5</summary>

##
[0.4.5](observability-0.4.4...observability-0.4.5)
(2025-10-06)


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * highlight.run bumped to 9.22.2
</details>

<details><summary>observability-node: 0.3.1</summary>

##
[0.3.1](observability-node-0.3.0...observability-node-0.3.1)
(2025-10-06)


### Bug Fixes

* Fix an issue where logging configuration could over-match for log
records without a message or severity.
([#261](#261))
([500a6bf](500a6bf))
</details>

<details><summary>session-replay: 0.4.5</summary>

##
[0.4.5](session-replay-0.4.4...session-replay-0.4.5)
(2025-10-06)


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * highlight.run bumped to 9.22.2
</details>

<details><summary>highlight.run: 9.22.2</summary>

##
[9.22.2](highlight.run-9.22.1...highlight.run-9.22.2)
(2025-10-06)


### Bug Fixes

* Fix an issue where logging configuration could over-match for log
records without a message or severity.
([#261](#261))
([500a6bf](500a6bf))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> Release updates across Go, Android, Node, and web packages with a
logging bug fix, Android metric instrument caching, and dependency bump
to highlight.run 9.22.2.
> 
> - **Releases**:
>   - `go` → `0.3.1`
> - Bug fix: prevent logging config over-matching for records without
message/severity.
> - Update `go/internal/metadata/metadata.go` InstrumentationVersion to
`0.3.1`.
>   - `sdk/@launchdarkly/observability-node` → `0.3.1`
>     - Bug fix: same logging over-match fix.
>   - `sdk/@launchdarkly/observability-android` → `0.10.0`
>     - Feature: cache OpenTelemetry metric instruments.
>   - `sdk/@launchdarkly/observability` → `0.4.5`
>     - Dependency: bump `highlight.run` to `9.22.2`.
>   - `sdk/@launchdarkly/session-replay` → `0.4.5`
>     - Dependency: bump `highlight.run` to `9.22.2`.
>   - `sdk/highlight-run` → `9.22.2`
>     - Bug fix: logging over-match.
> - **Manifest**: update `.release-please-manifest.json` versions
accordingly.
> - **Packages**: update related `package.json` versions to match.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
7ae2ff1. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
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