diff --git a/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.test.ts b/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.test.ts index 54ea860fb9..a6b7294866 100644 --- a/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.test.ts +++ b/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.test.ts @@ -18,10 +18,10 @@ import { BuildMetadataStorage } from './BuildMetadataStorage'; describe('BuildMetadataStorage', () => { it('should return build timestamp', () => { const newMetadataStorage = new BuildMetadataStorage('entityID123abc'); - newMetadataStorage.setEtag('etag123abc'); + newMetadataStorage.setLastUpdated(); - const timestamp = newMetadataStorage.getEtag(); + const timestamp = newMetadataStorage.getLastUpdated(); - expect(timestamp).toBe('etag123abc'); + expect(timestamp).toBeLessThanOrEqual(Date.now()); }); }); diff --git a/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.ts b/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.ts index a1bb918fae..ce42716e3b 100644 --- a/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.ts +++ b/plugins/techdocs-backend/src/DocsBuilder/BuildMetadataStorage.ts @@ -13,37 +13,42 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -type buildInfo = { - // Entity uid: etag - [key: string]: string; -}; -// TODO: Build info should be part of TechDocs storage, inside `techdocs_metadata.json` -// instead of in-memory storage of the Backstage instance. -// In case of multi-region Backstage deployments, or even using multiple Kubernetes pods, -// if each instance creates its separate build info in-memory, it will result in duplicate -// builds per instance. Also if the pod restarts, all the sites will have to be re-built. -const builds = {} as buildInfo; +// Entity uid: unix timestamp +const lastUpdatedRecord = {} as Record; /** - * Store timestamps of the most recent TechDocs build of each Entity. This is - * used to invalidate cache if the latest commit in the documentation source - * repository is later than the timestamp. + * Store timestamps of the most recent TechDocs update of each Entity. This is + * used to avoid checking for an update on each and every request to TechDocs. */ export class BuildMetadataStorage { - public entityUid: string; - private builds: buildInfo; + private entityUid: string; + private lastUpdatedRecord: Record; constructor(entityUid: string) { this.entityUid = entityUid; - this.builds = builds; + this.lastUpdatedRecord = lastUpdatedRecord; } - setEtag(etag: string): void { - this.builds[this.entityUid] = etag; + setLastUpdated(): void { + this.lastUpdatedRecord[this.entityUid] = Date.now(); } - getEtag(): string | undefined { - return this.builds[this.entityUid]; + getLastUpdated(): number | undefined { + return this.lastUpdatedRecord[this.entityUid]; } } + +/** + * Return false if a check for update has happened in last 60 seconds. + */ +export const shouldCheckForUpdate = (entityUid: string) => { + const lastUpdated = new BuildMetadataStorage(entityUid).getLastUpdated(); + if (lastUpdated) { + // The difference is in milliseconds + if (Date.now() - lastUpdated < 60 * 1000) { + return false; + } + } + return true; +}; diff --git a/plugins/techdocs-backend/src/DocsBuilder/builder.ts b/plugins/techdocs-backend/src/DocsBuilder/builder.ts index f42a708bb2..5564b7da45 100644 --- a/plugins/techdocs-backend/src/DocsBuilder/builder.ts +++ b/plugins/techdocs-backend/src/DocsBuilder/builder.ts @@ -13,8 +13,12 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +import { + Entity, + ENTITY_DEFAULT_NAMESPACE, + serializeEntityRef, +} from '@backstage/catalog-model'; import { NotModifiedError } from '@backstage/errors'; -import { Entity, serializeEntityRef } from '@backstage/catalog-model'; import { GeneratorBase, GeneratorBuilder, @@ -81,28 +85,33 @@ export class DocsBuilder { )}`, ); - // Use the in-memory storage for setting and getting etag for this entity. - const buildMetadataStorage = new BuildMetadataStorage( - this.entity.metadata.uid, - ); + // If available, use the etag stored in techdocs_metadata.json to + // check if docs are outdated and needs to be regenerated. + let storedEtag: string | undefined; + if (await this.publisher.hasDocsBeenGenerated(this.entity)) { + storedEtag = ( + await this.publisher.fetchTechDocsMetadata({ + namespace: this.entity.metadata.namespace ?? ENTITY_DEFAULT_NAMESPACE, + kind: this.entity.kind, + name: this.entity.metadata.name, + }) + ).etag; + } - // TODO: As of now, this happens on each and every request to TechDocs. - // In a high traffic environment, this will cause a lot of requests to the source code provider. - // After Async build is implemented https://github.com/backstage/backstage/issues/3717, - // make sure to limit checking for cache invalidation to once per minute or so. let preparedDir: string; - let etag: string; - + let newEtag: string; try { const preparerResponse = await this.preparer.prepare(this.entity, { - etag: buildMetadataStorage.getEtag(), + etag: storedEtag, }); preparedDir = preparerResponse.preparedDir; - etag = preparerResponse.etag; + newEtag = preparerResponse.etag; } catch (err) { if (err instanceof NotModifiedError) { // No need to prepare anymore since cache is valid. + // Set last check happened to now + new BuildMetadataStorage(this.entity.metadata.uid).setLastUpdated(); this.logger.debug( `Docs for ${serializeEntityRef( this.entity, @@ -142,7 +151,7 @@ export class DocsBuilder { outputDir, dockerClient: this.dockerClient, parsedLocationAnnotation, - etag, + etag: newEtag, }); // Remove Prepared directory since it is no longer needed. @@ -185,7 +194,7 @@ export class DocsBuilder { this.logger.debug(`Error removing generated directory ${error.message}`); } - // Store the latest build etag for the entity - new BuildMetadataStorage(this.entity.metadata.uid).setEtag(etag); + // Update the last check time for the entity + new BuildMetadataStorage(this.entity.metadata.uid).setLastUpdated(); } } diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index 834a3a78ef..95d8e57074 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -29,6 +29,7 @@ import Router from 'express-promise-router'; import { Knex } from 'knex'; import { Logger } from 'winston'; import { DocsBuilder } from '../DocsBuilder'; +import { shouldCheckForUpdate } from '../DocsBuilder/BuildMetadataStorage'; import { getEntityNameFromUrlPath } from './helpers'; type RouterOptions = { @@ -140,7 +141,11 @@ export async function createRouter({ // techdocs-backend will only try to build documentation for an entity if techdocs.builder is set to 'local' // If set to 'external', it will only try to fetch and assume that an external process (e.g. CI/CD pipeline // of the repository) is responsible for building and publishing documentation to the storage provider. - if (config.getString('techdocs.builder') === 'local') { + if ( + config.getString('techdocs.builder') === 'local' && + entity.metadata.uid && + shouldCheckForUpdate(entity.metadata.uid) + ) { const docsBuilder = new DocsBuilder({ preparers, generators, @@ -149,53 +154,36 @@ export async function createRouter({ logger, entity, }); + let foundDocs = false; switch (publisherType) { case 'local': - await docsBuilder.build(); - break; case 'awsS3': case 'azureBlobStorage': case 'openStackSwift': case 'googleGcs': - // This block should be valid for all external storage implementations. So no need to duplicate in future, + // This block should be valid for all storage implementations. So no need to duplicate in future, // add the publisher type in the list here. - if (!(await publisher.hasDocsBeenGenerated(entity))) { - logger.info( - 'No pre-generated documentation files found for the entity in the storage. Building docs...', - ); - await docsBuilder.build(); - // With a maximum of ~5 seconds wait, check if the files got published and if docs will be fetched - // on the user's page. If not, respond with a message asking them to check back later. - // The delay here is to make sure GCS/AWS/etc. registers newly uploaded files which is usually <1 second - let foundDocs = false; - for (let attempt = 0; attempt < 5; attempt++) { - if (await publisher.hasDocsBeenGenerated(entity)) { - foundDocs = true; - break; - } - await new Promise(r => setTimeout(r, 1000)); + await docsBuilder.build(); + // With a maximum of ~5 seconds wait, check if the files got published and if docs will be fetched + // on the user's page. If not, respond with a message asking them to check back later. + // The delay here is to make sure GCS/AWS/etc. registers newly uploaded files which is usually <1 second + for (let attempt = 0; attempt < 5; attempt++) { + if (await publisher.hasDocsBeenGenerated(entity)) { + foundDocs = true; + break; } - if (!foundDocs) { - logger.error( - 'Published files are taking longer to show up in storage. Something went wrong.', + await new Promise(r => setTimeout(r, 1000)); + } + if (!foundDocs) { + logger.error( + 'Published files are taking longer to show up in storage. Something went wrong.', + ); + res + .status(408) + .send( + 'Sorry! It is taking longer for the generated docs to show up in storage. Check back later.', ); - res - .status(408) - .send( - 'Sorry! It is taking longer for the generated docs to show up in storage. Check back later.', - ); - return; - } - } else { - logger.info( - 'Found pre-generated docs for this entity. Serving them.', - ); - // TODO: re-trigger build for cache invalidation. - // Add build info in techdocs_metadata.json and compare it against - // the etag/commit in the repository. - // Without this, docs will not be re-built once they have been generated. - // Although it is unconventional that anyone will face this issue - because - // if you have an external storage, you should be using CI/CD to build and publish docs. + return; } break; default: