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

[APM] Should this promise be retried? #182274

Open
Tracked by #182291
afharo opened this issue May 1, 2024 · 4 comments
Open
Tracked by #182291

[APM] Should this promise be retried? #182274

afharo opened this issue May 1, 2024 · 4 comments
Labels
Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team technical debt Improvement of the software architecture and operational architecture

Comments

@afharo
Copy link
Member

afharo commented May 1, 2024

The promises below might fail in case there's a glitch in ES connection and never retry.

await taskManagerStart.ensureScheduled({
id: APM_TELEMETRY_TASK_NAME,
taskType: APM_TELEMETRY_TASK_NAME,
schedule: {
interval: '720m',
},
scope: ['apm'],
params: {},
state: {},
});
try {
const currentData = (
await savedObjectsClient.get(
APM_TELEMETRY_SAVED_OBJECT_TYPE,
APM_TELEMETRY_SAVED_OBJECT_ID
)
).attributes as { kibanaVersion?: string };
if (currentData.kibanaVersion !== kibanaVersion) {
logger.debug(
`Stored telemetry is out of date. Task will run immediately. Stored: ${currentData.kibanaVersion}, expected: ${kibanaVersion}`
);
await taskManagerStart.runSoon(APM_TELEMETRY_TASK_NAME);
}
} catch (err) {
if (!SavedObjectsErrorHelpers.isNotFoundError(err)) {
logger.warn('Failed to fetch saved telemetry data.');
}
}

createApmTelemetry({
core,
getApmIndices: plugins.apmDataAccess.getApmIndices,
usageCollector: plugins.usageCollection,
taskManager: plugins.taskManager,
logger: this.logger,
kibanaVersion: this.initContext.env.packageInfo.version,
isProd: this.initContext.env.mode.prod,
}).catch(() => {});

apmIndicesPromise
.then((apmIndices) => {
plugins.home?.tutorials.registerTutorial(
tutorialProvider({
apmConfig: currentConfig,
apmIndices,
cloud: plugins.cloud,
isFleetPluginEnabled: !isEmpty(resourcePlugins.fleet),
})
);
})
.catch(() => {});

Is that OK? Should we apply any retry logic?

If we want to retry, p-retry could be a useful utility that's already present in the project.

@afharo afharo added the Team:APM All issues that need APM UI Team support label May 1, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:APM)

@dgieselaar
Copy link
Member

@afharo It's okay to not retry. We want to keep this as unintrusive as possible, to prevent any unnecessary load on the cluster.

@afharo
Copy link
Member Author

afharo commented May 2, 2024

@dgieselaar, thanks for confirming! FWIW, I created this issue for awareness. Closing the issue is perfectly valid.

Just to confirm about this case

apmIndicesPromise
.then((apmIndices) => {
plugins.home?.tutorials.registerTutorial(
tutorialProvider({
apmConfig: currentConfig,
apmIndices,
cloud: plugins.cloud,
isFleetPluginEnabled: !isEmpty(resourcePlugins.fleet),
})
);
})
.catch(() => {});

If the APM promise fails, the plugin won't register the tutorial

Wrt telemetry-related promises, if they fail, we won't have telemetry. But I understand we might be ok with that to avoid overloading the cluster.

@dgieselaar
Copy link
Member

@afharo ah sorry missed the last one! I think a retry makes sense there, but I will leave it up to @elastic/obs-ux-infra_services-team to figure out what is best there. Thanks for bringing it up in any case!

@smith smith added Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team technical debt Improvement of the software architecture and operational architecture and removed Team:APM All issues that need APM UI Team support labels May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team technical debt Improvement of the software architecture and operational architecture
Projects
None yet
Development

No branches or pull requests

4 participants