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

chore: marking trace, span related ids as non-groupable #93

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kotharironak
Copy link
Contributor

The 'groupable' field in attribute definition helps determine whether that attribute is used for grouping, mainly in a time-series context. Since span/trace IDs are unique values, it doesn't make much sense, as the time-series will mostly result in a count of 1.

This is also causing some performance issues and results in an error when used in conjunction with DistinctCountHLL. As part of this PR, we are moving them into the non-groupable category.

Copy link

codecov bot commented Nov 7, 2023

Codecov Report

Merging #93 (bc4b799) into main (7e82382) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               main      #93   +/-   ##
=========================================
  Coverage     74.10%   74.10%           
  Complexity       69       69           
=========================================
  Files            14       14           
  Lines           560      560           
  Branches         34       34           
=========================================
  Hits            415      415           
  Misses          128      128           
  Partials         17       17           
Flag Coverage Δ
integration 74.10% <ø> (ø)
unit 43.03% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

Copy link

github-actions bot commented Nov 7, 2023

Unit Test Results

  3 files  ±0    3 suites  ±0   5s ⏱️ -3s
11 tests ±0  11 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit bc4b799. ± Comparison against base commit 7e82382.

@@ -12,7 +12,7 @@ commands = [
fqn: Span.id,
key: id,
value_kind: TYPE_STRING,
groupable: true,
groupable: false,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do I need to add rollback block with action update to go back to true value?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, but also you shouldn't change a historical command. You want to add a new command with its own rollback that describes the desired and previous state.

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.

2 participants