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

feat: capture performance metrics from trace events #194

Merged
merged 11 commits into from Jun 1, 2021

Conversation

vigneshshanmugam
Copy link
Member

@vigneshshanmugam vigneshshanmugam commented Jan 20, 2021

  • fixes Capture relevant perf metrics to close the gap with RUM agent #120
  • Captures FirstContentfulPaint, LongestContentful Paint, Navigationorigin, Dom content loaded, Load event, Layout shift events, Cumulative Layout shift from chrome trace events
  • Replace trace flag with filmstrips in the CLI.
  • Writes to the JSON reporter as journey/metrics to reflect the trace output timing information.
  • Uses the Lighthouse Trace processor to extract the events specific to the current main process.
  • included Notice.txt which ppints to Lighthouse license as we are using their trace processor for extracting key metrics.

Trace output is in this format

{
  "type": "journey/metrics",
  "@timestamp": 1622578544529441,
  "journey": {
    "name": "check if input placeholder is correct",
    "id": "check if input placeholder is correct"
  },
  "root_fields": {
    "browser": {
      "experience": {
        "name": "navigationStart",
        "type": "mark",
        "start": 1014038.407051
      }
    },
    "os": {
      "platform": "darwin"
    },
    "package": {
      "name": "@elastic/synthetics",
      "version": "1.0.0-beta.2"
    }
  },
  "package_version": "1.0.0-beta.2"
}

start, end - Timings that are relative to step/journey start and end times.

@apmmachine
Copy link
Collaborator

apmmachine commented Jan 20, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Started by user Vignesh Shanmugam

  • Start Time: 2021-06-01T20:15:02.323+0000

  • Duration: 13 min 42 sec

  • Commit: 432e791

Test stats 🧪

Test Results
Failed 0
Passed 96
Skipped 0
Total 96

Trends 🧪

Image of Build Times

Image of Tests

Copy link
Contributor

@andrewvc andrewvc left a comment

Choose a reason for hiding this comment

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

Leaving a partial review here will follow up with a complete review ASAP

src/parse_args.ts Show resolved Hide resolved
src/sdk/trace-metrics.ts Show resolved Hide resolved
src/sdk/trace-metrics.ts Show resolved Hide resolved
src/sdk/trace-processor.ts Show resolved Hide resolved
__tests__/index.test.ts Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
__tests__/sdk/trace-metrics.test.ts Outdated Show resolved Hide resolved
__tests__/sdk/trace-metrics.test.ts Show resolved Hide resolved
src/common_types.ts Outdated Show resolved Hide resolved
src/core/gatherer.ts Outdated Show resolved Hide resolved
@andrewvc
Copy link
Contributor

FYI I did reach out to @drewpost and discuss the fact that metrics we collect are not aligned up with lighthouse or many other tools due to the fact that specific browser versions and platform differences will prevent numbers from lining up exactly.

Once this is merged we should be sure sure to document these differences and explain performance in the synthetics documentation. I've opened elastic/observability-docs#735 to track

@vigneshshanmugam
Copy link
Member Author

@andrewvc Thanks Andrew for taking care of that and the info.

Copy link
Contributor

@andrewvc andrewvc left a comment

Choose a reason for hiding this comment

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

LGTM however I'll leave it up to you to determine whether you want to move the fields out of the payload now or in a subsequent PR. We can't build GUI features based on this until we have proper elasticsearch fields defined in our mappings.

Copy link
Contributor

@andrewvc andrewvc left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Capture relevant perf metrics to close the gap with RUM agent
3 participants