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.

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

Flaky test LastValueAggregator #3572

Closed
Flarna opened this issue Jan 27, 2023 · 1 comment · Fixed by #3587
Closed

Flaky test LastValueAggregator #3572

Flarna opened this issue Jan 27, 2023 · 1 comment · Fixed by #3587
Assignees
Labels
bug Something isn't working internal priority:p4 Bugs and spec inconsistencies which do not fall into a higher prioritization

Comments

@Flarna
Copy link
Member

Flarna commented Jan 27, 2023

I have seen the LastValueAggregator failing several times in the past in various PRs. Rerun usually result in a green CI.

I assume this is some timing issue, maybe it's enough to just increase the timeout here.
Usually it's more stable to avoid relying on timeouts instead stub the needed trigger and ensure sequence but I'm not that familiar with metrics SDK.

Log output:

  1) LastValueAggregator
       merge
         return the newly sampled accumulation:

      AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
+ actual - expected ... Lines skipped

  LastValueAccumulation {
+   _current: 3,
-   _current: 4,
    sampleTime: [
...
      0
    ]
  }
      + expected - actual

       {
      -  "_current": 3
      +  "_current": 4
         "sampleTime": [
           1674779663
           975000000
         ]
      
      at Context.<anonymous> (test\aggregator\LastValue.test.ts:62:14)

see e.g.
https://github.com/open-telemetry/opentelemetry-js/actions/runs/4022668001/jobs/6912651597
https://github.com/open-telemetry/opentelemetry-js/actions/runs/4020374096/jobs/6908235579
https://github.com/open-telemetry/opentelemetry-js/actions/runs/4019619610/jobs/6906675388

@Flarna Flarna added bug Something isn't working triage labels Jan 27, 2023
@legendecas
Copy link
Member

The timeout is used to verify the sample time of the accumulation. It is not supposed to be related to timing issue. I'll take a look at this problem.

@pichlermarc pichlermarc added internal priority:p4 Bugs and spec inconsistencies which do not fall into a higher prioritization and removed triage labels Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working internal priority:p4 Bugs and spec inconsistencies which do not fall into a higher prioritization
Projects
None yet
3 participants