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

Implement spans for metrics profiling #1268

Merged
merged 9 commits into from
Oct 10, 2024

Conversation

roby2014
Copy link
Member

@roby2014 roby2014 commented Jun 20, 2024

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 roby2014 added this to the 0.3 Standalone Editor milestone Jun 20, 2024
@roby2014 roby2014 self-assigned this Jun 20, 2024
@roby2014 roby2014 linked an issue Jun 20, 2024 that may be closed by this pull request
@github-actions github-actions bot added A-Core B-Telemetry S-Triage Issues whose priority still has to be figured out labels Jun 20, 2024
Copy link
Contributor

github-actions bot commented Jun 20, 2024

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

Copy link

codecov bot commented Jun 20, 2024

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.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ 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
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ 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 RiscadoA modified the milestones: 0.3, 0.4 - ??? Aug 2, 2024
@roby2014 roby2014 force-pushed the 1265-implement-spans-for-metrics-+-profiling branch from fd5ab56 to 3933444 Compare August 21, 2024 13:10
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

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

core/tests/data/des/json.cpp Show resolved Hide resolved
@roby2014 roby2014 force-pushed the 1265-implement-spans-for-metrics-+-profiling branch from 3933444 to d8eba43 Compare August 21, 2024 13:22
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ 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 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 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
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ 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 roby2014 force-pushed the 1265-implement-spans-for-metrics-+-profiling branch from 9a38bd9 to c3fff51 Compare September 5, 2024 09:03
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ 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 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
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ 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 roby2014 force-pushed the 1265-implement-spans-for-metrics-+-profiling branch from 314df1f to 6664d07 Compare September 5, 2024 09:21
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ 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 roby2014 force-pushed the 1265-implement-spans-for-metrics-+-profiling branch from 0bee3c8 to f966401 Compare September 29, 2024 17:34
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

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

core/include/cubos/core/tel/tracing.hpp Show resolved Hide resolved
core/include/cubos/core/tel/tracing.hpp Outdated Show resolved Hide resolved
@roby2014 roby2014 force-pushed the 1265-implement-spans-for-metrics-+-profiling branch from f966401 to eae508b Compare September 30, 2024 18:53
Copy link
Contributor

@diogomsmiranda diogomsmiranda left a comment

Choose a reason for hiding this comment

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

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 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
Copy link
Member

@RiscadoA RiscadoA left a comment

Choose a reason for hiding this comment

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

LGTM! Ty for the crunch 🙏

@roby2014 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 roby2014 force-pushed the 1265-implement-spans-for-metrics-+-profiling branch from 7aa7c6c to 9189a09 Compare October 10, 2024 19:06
@roby2014 roby2014 merged commit ff78548 into main Oct 10, 2024
11 checks passed
@roby2014 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 Issues whose priority still has to be figured out
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement spans for metrics + profiling
4 participants