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

[Telemetry] Possible race condition in stack_stats search reporting #89744

Closed
thesmallestduck opened this issue Jan 29, 2021 · 4 comments · Fixed by #91230
Closed

[Telemetry] Possible race condition in stack_stats search reporting #89744

thesmallestduck opened this issue Jan 29, 2021 · 4 comments · Fixed by #91230
Assignees
Labels
Feature:Telemetry PR sent Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@thesmallestduck
Copy link

I believe the metrics collection code found here can result in a possible race condition:

try {
const response = await repository.get<Usage>(SAVED_OBJECT_ID, SAVED_OBJECT_ID);
attributes = response.attributes;
} catch (e) {
doesSavedObjectExist = false;
attributes = {
successCount: 0,
errorCount: 0,
averageDuration: 0,
};
}
attributes[eventType]++;
// Only track the average duration for successful requests
if (eventType === 'successCount') {
attributes.averageDuration =
((duration ?? 0) + (attributes.averageDuration ?? 0)) / (attributes.successCount ?? 1);
}
try {
if (doesSavedObjectExist) {
await repository.update(SAVED_OBJECT_ID, SAVED_OBJECT_ID, attributes);
} else {
await repository.create(SAVED_OBJECT_ID, attributes, { id: SAVED_OBJECT_ID });
}
} catch (e) {
// Version conflict error, swallow
}
};
};

This could result in telemetry data that is inaccurate. I believe moving the increment and average calculation to thread safe implementations should fix it.

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-telemetry (Team:KibanaTelemetry)

@thesmallestduck thesmallestduck added bug Fixes for quality problems that affect the customer experience and removed Feature:Telemetry Team:KibanaTelemetry labels Jan 29, 2021
@alexfrancoeur alexfrancoeur added Feature:Telemetry Team:AppServices and removed bug Fixes for quality problems that affect the customer experience labels Jan 29, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServices)

@afharo
Copy link
Member

afharo commented Jan 29, 2021

Good call @thesmallestduck! SOs API now provides an improved incrementCounter API. Maybe this is a good fit.

@lukasolson lukasolson self-assigned this Feb 18, 2021
@lukeelmers lukeelmers added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Oct 1, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Telemetry PR sent Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants