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.

Sign up for GitHub

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

Jump to bottom

Implement spans for metrics profiling #1268

Merged

roby2014 merged 9 commits into main from 1265-implement-spans-for-metrics-+-profiling

Oct 10, 2024

Member

roby2014 commented Jun 20, 2024 •

edited

Loading

Description

See linked issue for proposed implementation, but in short:

Create new namespace tel with metrics, tracing and logging.
Create a global "telemetry" level for logs and spans.
Create span utilities to keep track of scopes and execution times (for now).
Associate metrics and logs to spans.

Checklist

Self-review changes.
Evaluate impact on the documentation.
Ensure test coverage.
Write new samples.
Add entry to the changelog's unreleased section.

roby2014 added this to the 0.3 Standalone Editor milestone

roby2014 self-assigned this

roby2014 linked an issue

that may be closed by this pull request

Implement spans for metrics + profiling #1265

Closed

github-actions bot added A-Core B-Telemetry S-Triage labels

Contributor

github-actions bot commented Jun 20, 2024 •

edited

Loading

PR Preview Action v1.4.8
🚀 Deployed preview to https://GameDevTecnico.github.io/cubos/preview/pr-1268/
on branch `gh-pages` at 2024-10-10 18:23 UTC

codecov bot commented Jun 20, 2024 •

edited

Loading

Codecov Report

Attention: Patch coverage is 71.32353% with 39 lines in your changes missing coverage. Please review.

Project coverage is 40.31%. Comparing base (86c062d) to head (9189a09).
Report is 9 commits behind head on main.

Files with missing lines	Patch %	Lines
engine/src/tools/console/plugin.cpp	0.00%	33 Missing ⚠️
core/src/tel/logging.cpp	75.00%	4 Missing ⚠️
core/src/tel/tracing.cpp	93.33%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1268      +/-   ##
==========================================
+ Coverage   40.20%   40.31%   +0.11%     
==========================================
  Files         432      435       +3     
  Lines       30869    30908      +39     
==========================================
+ Hits        12411    12462      +51     
+ Misses      18458    18446      -12

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

github-actions bot requested changes

View reviewed changes

Contributor

github-actions bot left a comment

⚠️ Clang-Tidy found issue(s) with the introduced code (1/2)

core/include/cubos/core/tel/tracing.hpp Outdated Show resolved Hide resolved

core/src/tel/tracing.cpp Outdated Show resolved Hide resolved

core/include/cubos/core/tel/tracing.hpp Outdated Show resolved Hide resolved

core/include/cubos/core/tel/tracing.hpp Outdated Show resolved Hide resolved

core/include/cubos/core/tel/tracing.hpp Outdated Show resolved Hide resolved

core/src/tel/tracing.cpp Outdated Show resolved Hide resolved

core/src/tel/tracing.cpp Outdated Show resolved Hide resolved

core/src/tel/tracing.cpp Show resolved Hide resolved

core/src/tel/tracing.cpp Outdated Show resolved Hide resolved

core/src/tel/tracing.cpp Outdated Show resolved Hide resolved

github-actions bot previously requested changes

View reviewed changes

Contributor

github-actions bot left a comment

⚠️ Clang-Tidy found issue(s) with the introduced code (2/2)

core/src/tel/tracing.cpp Outdated Show resolved Hide resolved

core/src/tel/tracing.cpp Outdated Show resolved Hide resolved

RiscadoA modified the milestones: 0.3, 0.4 - ???

roby2014 force-pushed the 1265-implement-spans-for-metrics-+-profiling branch from fd5ab56 to 3933444 Compare

August 21, 2024 13:10

github-actions bot reviewed

View reviewed changes

Contributor

github-actions bot left a comment

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

core/tests/data/des/json.cpp Show resolved Hide resolved

roby2014 force-pushed the 1265-implement-spans-for-metrics-+-profiling branch from 3933444 to d8eba43 Compare

August 21, 2024 13:22

github-actions bot reviewed

View reviewed changes

Contributor

github-actions bot left a comment

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

core/src/tel/tracing.cpp Show resolved Hide resolved

core/src/tel/tracing.cpp Outdated Show resolved Hide resolved

core/src/tel/tracing.cpp Outdated Show resolved Hide resolved

core/include/cubos/core/tel/tracing.hpp Outdated Show resolved Hide resolved

roby2014 force-pushed the 1265-implement-spans-for-metrics-+-profiling branch 3 times, most recently from e6ad94e to 5ff571e Compare

August 21, 2024 14:36

roby2014 force-pushed the 1265-implement-spans-for-metrics-+-profiling branch 4 times, most recently from 8809e83 to 9a38bd9 Compare

September 5, 2024 08:58

github-actions bot reviewed

View reviewed changes

Contributor

github-actions bot left a comment

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

core/src/tel/tracing.cpp Show resolved Hide resolved

core/src/tel/tracing.cpp Show resolved Hide resolved

core/src/tel/tracing.cpp Outdated Show resolved Hide resolved

core/include/cubos/core/tel/tracing.hpp Outdated Show resolved Hide resolved

core/src/tel/tracing.cpp Show resolved Hide resolved

core/src/tel/tracing.cpp Outdated Show resolved Hide resolved

core/src/tel/tracing.cpp Show resolved Hide resolved

core/src/tel/tracing.cpp Show resolved Hide resolved

roby2014 force-pushed the 1265-implement-spans-for-metrics-+-profiling branch from 9a38bd9 to c3fff51 Compare

September 5, 2024 09:03

github-actions bot reviewed

View reviewed changes

Contributor

github-actions bot left a comment

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

core/src/tel/tracing.cpp Outdated Show resolved Hide resolved

core/src/tel/tracing.cpp Outdated Show resolved Hide resolved

core/src/tel/tracing.cpp Outdated Show resolved Hide resolved

core/src/tel/tracing.cpp Outdated Show resolved Hide resolved

roby2014 force-pushed the 1265-implement-spans-for-metrics-+-profiling branch 2 times, most recently from bc168d7 to 314df1f Compare

September 5, 2024 09:14

github-actions bot reviewed

View reviewed changes

Contributor

github-actions bot left a comment

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

core/include/cubos/core/tel/tracing.hpp Outdated Show resolved Hide resolved

core/src/tel/tracing.cpp Outdated Show resolved Hide resolved

core/src/tel/tracing.cpp Outdated Show resolved Hide resolved

core/src/tel/tracing.cpp Outdated Show resolved Hide resolved

roby2014 force-pushed the 1265-implement-spans-for-metrics-+-profiling branch from 314df1f to 6664d07 Compare

September 5, 2024 09:21

github-actions bot reviewed

View reviewed changes

Contributor

github-actions bot left a comment

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

core/tests/tel/tracing.cpp Outdated Show resolved Hide resolved

core/samples/tel/tracing/main.cpp Show resolved Hide resolved

core/samples/tel/tracing/main.cpp Show resolved Hide resolved

roby2014 force-pushed the 1265-implement-spans-for-metrics-+-profiling branch from 0bee3c8 to f966401 Compare

September 29, 2024 17:34

github-actions bot reviewed

View reviewed changes

Contributor

github-actions bot left a comment

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

core/include/cubos/core/tel/tracing.hpp Show resolved Hide resolved

RiscadoA requested changes

View reviewed changes

core/include/cubos/core/tel/tracing.hpp Outdated Show resolved Hide resolved

roby2014 force-pushed the 1265-implement-spans-for-metrics-+-profiling branch from f966401 to eae508b Compare

September 30, 2024 18:53

diogomsmiranda approved these changes

View reviewed changes

Contributor

diogomsmiranda left a comment •

edited

Loading

I don't see much code of this part of CUBOS so can't be of much help, but looks like most of files changed were just refactor of the code to fit the new namespace. Found nothing very wrong when looking at the implementation and docs.
If all the other requested changes were already addressed then LGTM!

Very useful feature aswell!!

roby2014 force-pushed the 1265-implement-spans-for-metrics-+-profiling branch 7 times, most recently from b8f4a46 to 174de77 Compare

October 7, 2024 19:05

RiscadoA approved these changes

View reviewed changes

Member

RiscadoA left a comment

LGTM! Ty for the crunch 🙏

RiscadoA dismissed github-actions[bot]’s stale review

October 9, 2024 09:01

Outdated

Dageus approved these changes

View reviewed changes

roby2014 force-pushed the 1265-implement-spans-for-metrics-+-profiling branch 2 times, most recently from 62242f8 to 7aa7c6c Compare

October 10, 2024 18:43

roby2014 and others added 9 commits

October 10, 2024 20:06


          docs(core): move logger to telemetry namespace

970f169


          docs(core): move metrics to telemetry namespace

0c3c54e


          feat(core): impl telemetry global level

05b22ab


          feat(core): implement tracing

9d297af


          docs(core): move logger, metrics and tracing samples to telemetry

c17ca01


          test(telemetry): add tracing tests

230dbc4


          fix(core): fix metrics

dbb49b1


          feat(core): add tracing sample

1f31030


          docs: update CHANGELOG.md

9189a09

roby2014 force-pushed the 1265-implement-spans-for-metrics-+-profiling branch from 7aa7c6c to 9189a09 Compare

October 10, 2024 19:06

roby2014 merged commit ff78548 into main

11 checks passed

roby2014 deleted the 1265-implement-spans-for-metrics-+-profiling branch

October 10, 2024 19:33

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Core B-Telemetry S-Triage