Skip to content

RUM-10316: using common-schema for common object generation#2794

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

RUM-10316: using common-schema for common object generation#2794
satween merged 1 commit into
feature/v3from
tvaleev/feature/RUM-10316-aligning_device_and_os

Conversation

@satween

@satween satween commented Jul 21, 2025

Copy link
Copy Markdown
Contributor

What does this PR do?

This PR continues changes started in #2793

There are several changes were mage under this PR:

  1. cloneCommonSchema task was added for -logs and -trace modules - making possible to clone most recent schema file from schema repository
  2. generateLogModelsFromJson and generateTraceModelFromJson tasks were updated to ignore objects from _common_schema.json from being generated directly
  3. span-schema.json and log-schema.json files were updated to support property referencing added in RUM-10316 - properties referencing support #2793
  4. Forgery factories, tests and mappers were updated in order to support new event structure

Motivation

We aiming to align event schema for v3 on both platforms.
Similar changes were made for ios DataDog/dd-sdk-ios#2322

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 15:08
@codecov-commenter

codecov-commenter commented Jul 21, 2025

Copy link
Copy Markdown

Codecov Report

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

Additional details and impacted files
@@              Coverage Diff               @@
##           feature/v3    #2794      +/-   ##
==============================================
- Coverage       67.62%   67.59%   -0.03%     
==============================================
  Files             818      818              
  Lines           30745    30763      +18     
  Branches         5179     5180       +1     
==============================================
+ Hits            20789    20792       +3     
- Misses           8564     8583      +19     
+ Partials         1392     1388       -4     
Files with missing lines Coverage Δ
...android/log/internal/domain/DatadogLogGenerator.kt 97.87% <100.00%> (+0.23%) ⬆️
...trace/internal/domain/event/BaseSpanEventMapper.kt 100.00% <100.00%> (+4.88%) ⬆️
...al/domain/event/CoreTracerSpanToSpanEventMapper.kt 93.33% <100.00%> (+2.22%) ⬆️
...e/internal/domain/event/DdSpanToSpanEventMapper.kt 92.16% <100.00%> (ø)

... and 33 files with indirect coverage changes

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

extension.apply {
clone(
"https://github.com/DataDog/rum-events-format.git",
"schemas/rum",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just out of curiosity. Will it work if I write here schemas/rum/_common-schema.json without specifying all other files in excludedPrefixes?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it would but it will download all those files that is unnecessary for logs schema.
Specifying single file is not supported by clone gradle plugin at this moment

@aleksandr-gringauz

Copy link
Copy Markdown
Contributor

Is my understanding correct that another solution would be just to change schemas for -logs and -traces manually?

Yes, there will not be a single source of truth but we would avoid all complicated changes here.

@aleksandr-gringauz

Copy link
Copy Markdown
Contributor

And where does schema for logs for example actually come from (tbh I don't know)? The way I see it we should just use that (whatever it is). We shouldn't on our side decide that "oh actually this should be the same as in RUM". Logs might know nothing about RUM backend.

@satween

satween commented Jul 28, 2025

Copy link
Copy Markdown
Contributor Author

Is my understanding correct that another solution would be just to change schemas for -logs and -traces manually?

Yes, there will not be a single source of truth but we would avoid all complicated changes here.

Well, not exactly. We need to have same schema as iOS has. So in case if we just manually adjust *.json files we still gonna have lot of changes in generated code and as well as in mapper logic. So the only difference will be actually in schema file. (And we could avoid adding clone plugin integration). But on the other hand every schema change will force us manually compare changes and align schema, which might lead to potential issues

@satween

satween commented Jul 28, 2025

Copy link
Copy Markdown
Contributor Author

And where does schema for logs for example actually come from (tbh I don't know)? The way I see it we should just use that (whatever it is). We shouldn't on our side decide that "oh actually this should be the same as in RUM". Logs might know nothing about RUM backend.

Ageed, and we don't actually use RUM schema for logs. We use RUM schema for device and os object. I think it's worth to fix the mess in schema repo and extract some common files which will have all common object, that we could rely on from different features. But such change gonna require to adjust browser and iOS SDKs which is too much for this task at the moment

Base automatically changed from tvaleev/feature/RUM-10316-properties-ref-support to feature/v3 July 28, 2025 11:22
@satween
satween dismissed aleksandr-gringauz’s stale review July 28, 2025 11:22

The base branch was changed.

@satween
satween merged commit 5957478 into feature/v3 Jul 28, 2025
25 checks passed
@satween
satween deleted the tvaleev/feature/RUM-10316-aligning_device_and_os branch July 28, 2025 14:08
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.

4 participants