Skip to content
This repository has been archived by the owner on May 3, 2024. It is now read-only.

Feature/ssr duration logging #1021

Closed
wants to merge 10 commits into from
Closed

Conversation

PixnBits
Copy link
Contributor

@PixnBits PixnBits commented Jun 9, 2023

Description

builds off of #991 and logs the observed render duration by URL, adding a warning entry

Motivation and Context

the summary metric is useful, but it is important to be able to distinguish which routes might be taking most of the CPU and/or contributing to event loop lag

this is a draft PR as a proposal, another idea is to instead build off of #1014 and log the trace when a synchronous time threshold is crossed

How Has This Been Tested?

ran locally

$ npm run build:server && \
ONE_CONFIG_ENV=e1 \
ONE_EXPERIMENTAL_RENDER_WARN_THRESHOLD=0.0021 \
npm start -- \
  --trace-warnings \
  --root-module-name=frank-lloyd-root \
  --log-format verbose \
  --log-level info
...
info: took 0.003134492s to renderForString path /
warn: render warning threshold of 0.0021s met or exceeded: took 0.003134492s to renderForString path /
...

will add tests if we want to go this route
and fill out the rest of this description 😅

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (adding or updating documentation)
  • Dependency update
  • Security update

Checklist:

  • My change requires a change to the documentation and I have updated the documentation accordingly.
  • These changes should be applied to a maintenance branch.
  • This change requires cross browser checks.
  • Performance tests should be ran against the server prior to merging.
  • This change impacts caching for client browsers.
  • This change impacts HTTP headers.
  • This change adds additional environment variable requirements for One App users.
  • I have added the Apache 2.0 license header to any new files created.

What is the Impact to Developers Using One App?

@@ -51,6 +51,71 @@ const getModuleData = async ({ dispatch, modules }) => {

const getModuleDataBreaker = createCircuitBreaker(getModuleData);

async function reactRender({ renderProps, store, requestURL }) {
Copy link
Contributor Author

@PixnBits PixnBits Jun 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh noes

src/server/plugins/reactHtml/createRequestHtmlFragment.jsx
  61:35  error  Async arrow function has a complexity of 13. Maximum allowed is 11  complexity

much refactoring :sad_trombone: (hence the changes in this specific commit, 0d572a7)

@github-actions
Copy link
Contributor

github-actions bot commented Jun 9, 2023

Size Change: 0 B

Total Size: 687 kB

ℹ️ View Unchanged
Filename Size
./build/app/app.js 164 kB
./build/app/app~vendors.js 389 kB
./build/app/runtime.js 7.07 kB
./build/app/service-worker-client.js 7.26 kB
./build/app/vendors.js 120 kB

compressed-size-action

@JAdshead
Copy link
Contributor

closing for http request metrics. #1034

@JAdshead JAdshead closed this Jun 29, 2023
@JAdshead JAdshead deleted the feature/ssr-duration-logging branch September 15, 2023 20:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants