Skip to content

[chore]Fix time skew in TestLRUSetLifeTime using epsilon assertion#43146

Merged
mx-psi merged 2 commits into
open-telemetry:mainfrom
lahsivjar:lrutestfix-take2
Oct 6, 2025
Merged

[chore]Fix time skew in TestLRUSetLifeTime using epsilon assertion#43146
mx-psi merged 2 commits into
open-telemetry:mainfrom
lahsivjar:lrutestfix-take2

Conversation

@lahsivjar
Copy link
Copy Markdown
Member

@lahsivjar lahsivjar commented Oct 6, 2025

Description

TestLRUSetLifeTime is a flaky test that we have taken a couple of stabs to fix in the past however we haven't been successful. I have tried reproducing the error in my local but failed. Looking into the code for go-freelru I discovered that the library uses wall clock (and not monotonic clock) for expiring items which might be the root cause of the flakiness (elastic/go-freelru#1). In an attempt to fix this, I have migrated to assert.InEpsilon and tightened the bounds for AssertEventually to about 1% delta.

Link to tracking issue

Fixes #42538

(edit: added by @mx-psi)

Fixes #42670
Fixes #42413

Testing

N/A

Documentation

N/A

Copy link
Copy Markdown
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM, I added a couple more issues to the "Fixes" list

@mx-psi mx-psi merged commit e94b501 into open-telemetry:main Oct 6, 2025
197 of 199 checks passed
@github-actions github-actions Bot added this to the next release milestone Oct 6, 2025
graphaelli pushed a commit to graphaelli/opentelemetry-collector-contrib that referenced this pull request Oct 6, 2025
…pen-telemetry#43146)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

`TestLRUSetLifeTime` is a flaky test that we have taken a couple of
stabs to fix in the past however we haven't been successful. I have
tried reproducing the error in my local but failed. Looking into the
code for go-freelru I discovered that the library uses wall clock (and
not monotonic clock) for expiring items which might be the root cause of
the flakiness (elastic/go-freelru#1). In an
attempt to fix this, I have migrated to `assert.InEpsilon` and tightened
the bounds for `AssertEventually` to about 1% delta.

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Fixes
open-telemetry#42538

(edit: added by @mx-psi)

Fixes open-telemetry#42670 
Fixes open-telemetry#42413

<!--Describe what testing was performed and which tests were added.-->
#### Testing
N/A

<!--Describe the documentation added.-->
#### Documentation
N/A

<!--Please delete paragraphs that you did not use before submitting.-->
@lahsivjar lahsivjar deleted the lrutestfix-take2 branch October 7, 2025 08:29
mashhurs pushed a commit to mashhurs/opentelemetry-collector-contrib that referenced this pull request Oct 9, 2025
…pen-telemetry#43146)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

`TestLRUSetLifeTime` is a flaky test that we have taken a couple of
stabs to fix in the past however we haven't been successful. I have
tried reproducing the error in my local but failed. Looking into the
code for go-freelru I discovered that the library uses wall clock (and
not monotonic clock) for expiring items which might be the root cause of
the flakiness (elastic/go-freelru#1). In an
attempt to fix this, I have migrated to `assert.InEpsilon` and tightened
the bounds for `AssertEventually` to about 1% delta.

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Fixes
open-telemetry#42538

(edit: added by @mx-psi)

Fixes open-telemetry#42670 
Fixes open-telemetry#42413

<!--Describe what testing was performed and which tests were added.-->
#### Testing
N/A

<!--Describe the documentation added.-->
#### Documentation
N/A

<!--Please delete paragraphs that you did not use before submitting.-->
tommyers-elastic pushed a commit to tommyers-elastic/opentelemetry-collector-contrib that referenced this pull request Oct 10, 2025
…pen-telemetry#43146)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

`TestLRUSetLifeTime` is a flaky test that we have taken a couple of
stabs to fix in the past however we haven't been successful. I have
tried reproducing the error in my local but failed. Looking into the
code for go-freelru I discovered that the library uses wall clock (and
not monotonic clock) for expiring items which might be the root cause of
the flakiness (elastic/go-freelru#1). In an
attempt to fix this, I have migrated to `assert.InEpsilon` and tightened
the bounds for `AssertEventually` to about 1% delta.

<!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. -->
#### Link to tracking issue
Fixes
open-telemetry#42538

(edit: added by @mx-psi)

Fixes open-telemetry#42670 
Fixes open-telemetry#42413

<!--Describe what testing was performed and which tests were added.-->
#### Testing
N/A

<!--Describe the documentation added.-->
#### Documentation
N/A

<!--Please delete paragraphs that you did not use before submitting.-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

3 participants