-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[exporter/signalfx] Fix correlation timeout bug (#9101)
* Fix SignalFx Exporter correlation timeout bug The SignalFx Exporter supports correlations between metrics and traces. This means that when a trace occurs from a given host, with dimensions like service and environment, these dimensions can be used to query metrics for these traces. These correlations are supposed to timeout after the stale_service_timeout amount of time passes, but they weren't. This fix makes sure correlations timeout so that metrics aren't linked with stale trace data. * Update CHANGELOG * Add doc file for timeutils package * Changes requested in PR - Add public type comment - Move implementing checker right under struct declaration - Add spaces between functions - Fix failing PR checks: Delay in tests failed in automation, add license block in test file. * Address racing test bug * Fix tests that had a race in them GitHub automation was failing unit tests because of a race detected. The solution was to add a mutex to checking and setting the variable in question. * Address lint and unit test failure * Fix linting failure and test timing Co-authored-by: Dmitrii Anoshin <anoshindx@gmail.com>
- Loading branch information
Showing
11 changed files
with
205 additions
and
63 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| // Copyright The OpenTelemetry Authors | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| // Package idutils provides a set of helper functions to convert ids. | ||
| // | ||
| // Functions in big_endian_converter.go help converting uint64 ids to TraceID | ||
| // and SpanID using big endian, and vice versa. | ||
| package timeutils // import "github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/timeutils" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| // Copyright The OpenTelemetry Authors | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package timeutils // import "github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/timeutils" | ||
|
|
||
| import "time" | ||
|
|
||
| // TTicker interface allows easier testing of Ticker related functionality | ||
| type TTicker interface { | ||
| // start sets the frequency of the Ticker and starts the periodic calls to OnTick. | ||
| Start(d time.Duration) | ||
| // OnTick is called when the Ticker fires. | ||
| OnTick() | ||
| // Stop firing the Ticker. | ||
| Stop() | ||
| } | ||
|
|
||
| // Implements TTicker and abstracts underlying time ticker's functionality to make usage | ||
| // simpler. | ||
| type PolicyTicker struct { | ||
| Ticker *time.Ticker | ||
| OnTickFunc func() | ||
| StopCh chan struct{} | ||
| } | ||
|
|
||
| // Ensure PolicyTicker implements TTicker interface | ||
| var _ TTicker = (*PolicyTicker)(nil) | ||
|
|
||
| func (pt *PolicyTicker) Start(d time.Duration) { | ||
| pt.Ticker = time.NewTicker(d) | ||
| pt.StopCh = make(chan struct{}) | ||
| go func() { | ||
| for { | ||
| select { | ||
| case <-pt.Ticker.C: | ||
| pt.OnTick() | ||
| case <-pt.StopCh: | ||
| return | ||
| } | ||
| } | ||
| }() | ||
| } | ||
|
|
||
| func (pt *PolicyTicker) OnTick() { | ||
| pt.OnTickFunc() | ||
| } | ||
|
|
||
| func (pt *PolicyTicker) Stop() { | ||
| close(pt.StopCh) | ||
| pt.Ticker.Stop() | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| // Copyright The OpenTelemetry Authors | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package timeutils | ||
|
|
||
| import ( | ||
| "sync" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| type testObj struct { | ||
| mu sync.Mutex | ||
| foo int | ||
| } | ||
|
|
||
| func (to *testObj) IncrementFoo() { | ||
| to.mu.Lock() | ||
| defer to.mu.Unlock() | ||
| to.foo++ | ||
| } | ||
|
|
||
| func checkFooValue(to *testObj, expectedValue int) bool { | ||
| to.mu.Lock() | ||
| defer to.mu.Unlock() | ||
| return to.foo == expectedValue | ||
| } | ||
|
|
||
| func TestPolicyTickerFails(t *testing.T) { | ||
| to := &testObj{foo: 0} | ||
| pTicker := &PolicyTicker{OnTickFunc: to.IncrementFoo} | ||
|
|
||
| // Tickers with a duration <= 0 should panic | ||
| assert.Panics(t, func() { pTicker.Start(time.Duration(0)) }) | ||
| assert.Panics(t, func() { pTicker.Start(time.Duration(-1)) }) | ||
| } | ||
|
|
||
| func TestPolicyTickerStart(t *testing.T) { | ||
| to := &testObj{foo: 0} | ||
| pTicker := &PolicyTicker{OnTickFunc: to.IncrementFoo} | ||
|
|
||
| // Make sure no ticks occur when we immediately stop the ticker | ||
| time.Sleep(100 * time.Millisecond) | ||
| assert.True(t, checkFooValue(to, 0), "Expected value: %d, actual: %d", 0, to.foo) | ||
| pTicker.Start(1 * time.Second) | ||
| pTicker.Stop() | ||
| assert.True(t, checkFooValue(to, 0), "Expected value: %d, actual: %d", 0, to.foo) | ||
| } | ||
|
|
||
| func TestPolicyTickerSucceeds(t *testing.T) { | ||
| // Start the ticker, make sure variable is incremented properly, | ||
| // also make sure stop works as expected. | ||
| to := &testObj{foo: 0} | ||
| pTicker := &PolicyTicker{OnTickFunc: to.IncrementFoo} | ||
|
|
||
| expectedTicks := 4 | ||
| defaultDuration := 500 * time.Millisecond | ||
| // Extra padding reduces the chance of a race happening here | ||
| padDuration := 200 * time.Millisecond | ||
| testSleepDuration := time.Duration(expectedTicks)*defaultDuration + padDuration | ||
|
|
||
| pTicker.Start(defaultDuration) | ||
| time.Sleep(testSleepDuration) | ||
| assert.True(t, checkFooValue(to, expectedTicks), "Expected value: %d, actual: %d", expectedTicks, to.foo) | ||
|
|
||
| pTicker.Stop() | ||
| // Since these tests are time sensitive they can be flaky. By getting the count | ||
| // after stopping, we can still test to make sure it's no longer being incremented, | ||
| // without requiring there by no OnTick calls between the sleep call and stopping. | ||
| time.Sleep(defaultDuration) | ||
| expectedTicks = to.foo | ||
|
|
||
| time.Sleep(testSleepDuration) | ||
| assert.True(t, checkFooValue(to, expectedTicks), "Expected value: %d, actual: %d", expectedTicks, to.foo) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
Oops, something went wrong.