Skip to content

RUM-10316 - properties referencing support#2793

Merged
satween merged 1 commit into
feature/v3from
tvaleev/feature/RUM-10316-properties-ref-support
Jul 28, 2025
Merged

RUM-10316 - properties referencing support#2793
satween merged 1 commit into
feature/v3from
tvaleev/feature/RUM-10316-properties-ref-support

Conversation

@satween

@satween satween commented Jul 21, 2025

Copy link
Copy Markdown
Contributor

What does this PR do?

In order to align os and device properties across different events we need to have single source of truth for device and os objects. Current JsonSchemaReader supports referencing only if referenced object was declared in definitions object.

This PR adds support for referencing object declared in properties. Note that only whole object referencing is supported because referencing by id requires name resolution, which makes referenced type a bit different every time it referenced.

see /external_nested_description_properties.json for example of new syntax supported

Motivation

We need this change in order to use rum/_common_schema.json as a source for device and os objects.

Additional Notes

Anything else we should know when reviewing?

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

@satween
satween requested review from a team as code owners July 21, 2025 14:22
@codecov-commenter

codecov-commenter commented Jul 21, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.62%. Comparing base (7e7fae2) to head (819ecb7).
⚠️ Report is 306 commits behind head on feature/v3.

Additional details and impacted files 
@@              Coverage Diff               @@
##           feature/v3    #2793      +/-   ##
==============================================
+ Coverage       67.54%   67.62%   +0.08%     
==============================================
  Files             818      818              
  Lines           30747    30745       -2     
  Branches         5178     5179       +1     
==============================================
+ Hits            20767    20789      +22     
+ Misses           8578     8564      -14     
+ Partials         1402     1392      -10

see 41 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@0xnm

0xnm commented Jul 21, 2025

Copy link
Copy Markdown
Member

Can you please give more details which referenced properties are needed to be supported? Indeed, device is declared in the common schema, but events generated for views, actions, etc. already have device and os properties in the model.

Do you mean they need to be referenced from the Logs and Traces schema instead of direct declaration there?

@satween satween mentioned this pull request Jul 21, 2025
Merged
3 tasks
@satween

satween commented Jul 21, 2025

Copy link
Copy Markdown
Contributor Author

Can you please give more details which referenced properties are needed to be supported? Indeed, device is declared in the common schema, but events generated for views, actions, etc. already have device and os properties in the model.

Do you mean they need to be referenced from the Logs and Traces schema instead of direct declaration there?

Yes, thats correct, see #2794 for more details.

@satween
satween merged commit c038e65 into feature/v3 Jul 28, 2025
26 checks passed
@satween
satween deleted the tvaleev/feature/RUM-10316-properties-ref-support branch July 28, 2025 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mariusc83 mariusc83 mariusc83 approved these changes
@jonathanmos jonathanmos jonathanmos approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@satween @codecov-commenter @0xnm @mariusc83 @jonathanmos