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

fix: perf relative traces and values #306

Merged
merged 7 commits into from
Jun 14, 2021

Conversation

vigneshshanmugam
Copy link
Member

@vigneshshanmugam vigneshshanmugam commented Jun 10, 2021

  • fix Improve mapping for additional metrics #303
  • Relative traces and metrics are grouped under two keys now to provide clear distinction.
  • Grouped all traces under browser.relative_trace to keep the mapping similar and these traces are mark or measure events with start and end time denoting the time when particular event (LCP, FCP, etc) occurred during the waterfall.
  • All distinct values like FCP, LCP, CLS, etc are grouped under browser.experience.<metric-name> to keep the querying easier in the UI and useful for displaying values in step detail page.

Relative trace events

{
  "type": "journey/metrics",
  "@timestamp": 1623300251603855.2,
  "journey": {
    "name": "check if title is present",
    "id": "check if title is present"
  },
  "root_fields": {
    "browser": {
      "relative_trace": {
        "name": "firstContentfulPaint",
        "type": "mark",
        "start": {
          "us": 142089104934
        }
      }
    },
  }
}

{
  "type": "journey/metrics",
  "journey": {
    "name": "inline",
    "id": "inline"
  },
  "root_fields": {
    "browser": {
      "relative_trace": {
        "name": "widget_core:widget_loaded",
        "type": "measure",
        "start": {
          "us": 142095075042
        },
        "end": {
          "us": 142095312098
        }
      }
    },
  }
}

Experience metrics

{
  "type": "journey/metrics",
  "@timestamp": 1623300251603949.5,
  "journey": {
    "name": "check if title is present",
    "id": "check if title is present"
  },
  "root_fields": {
    "browser": {
      "experience": {
        "cls": 0,
        "fcp": {
          "us": 189382
        },
        "lcp": {
          "us": 189382
        },
        "dcl": {
          "us": 209052
        },
        "load": {
          "us": 209115
        }
      }
    },
}

@apmmachine
Copy link

apmmachine commented Jun 10, 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: Pull request #306 updated

  • Start Time: 2021-06-14T17:21:46.528+0000

  • Duration: 13 min 58 sec

  • Commit: d3fab05

Test stats 🧪

Test Results
Failed 0
Passed 109
Skipped 0
Total 109

Trends 🧪

Image of Build Times

Image of Tests

@vigneshshanmugam vigneshshanmugam marked this pull request as ready for review June 10, 2021 04:41
const events = trace.mainThreadEvents;
const { score, event } = this.computeCLSValue(events);
Copy link
Member Author

Choose a reason for hiding this comment

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

all CLS relative trace events will be added as part of the work in #301

src/common_types.ts Outdated Show resolved Hide resolved
examples/todos/basic.journey.ts Outdated Show resolved Hide resolved
src/sdk/trace-metrics.ts Outdated Show resolved Hide resolved
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.

Found some more missing units, otherwise got. Looking better!

src/helpers.ts Outdated Show resolved Hide resolved
src/reporters/json.ts Show resolved Hide resolved
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, so much cleaner with easy to read units :)

@vigneshshanmugam vigneshshanmugam merged commit 97d7977 into elastic:master Jun 14, 2021
@vigneshshanmugam vigneshshanmugam deleted the update-ds branch June 14, 2021 22:17
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.

Improve mapping for additional metrics
5 participants