Skip to content

RUM-9521 Move RUM models to DatadogInternal #2272

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
Merged
dd-mergequeue merged 2 commits into from
Apr 28, 2025
Merged

RUM-9521 Move RUM models to DatadogInternal #2272

dd-mergequeue merged 2 commits into from
Apr 28, 2025

Conversation

@maxep
Copy link
Member

@maxep maxep commented Apr 24, 2025

What and why?

Models that are shared between modules must be shared in DatadogInternal. That is the case with RUM generated models.
This PR changes the generator destination for RUM schema.

How?

  • Change generator destination to DatadogInternal when generating RUM models.
  • Update imports accordingly
  • Re-export events used in mappers from RUM module.
  • Move/Update mocks accordingly

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference
  • Add CHANGELOG entry for user facing changes
  • Add Objective-C interface for public APIs (see our guidelines (internal) and run make api-surface)

@maxep maxep requested review from a team as code owners April 24, 2025 11:49
@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented Apr 24, 2025 •
edited
Loading

Datadog Report

Branch report: maxep/RUM-9521/change-rum-model-genetor-destination
Commit report: f1a25e8
Test service: dd-sdk-ios

✅ 0 Failed, 1278 Passed, 2626 Skipped, 3m 7.73s Total duration (2m 17.66s time saved)
❄️ 1 New Flaky

New Flaky Tests (1)

  • testItDoesSamplePeriodicallyWithLowFrequency - VitalInfoSamplerTests - Last Failure

    Expand for error 
     
 Assertion Failure at VitalInfoSamplerTests.swift:92: XCTAssertEqual failed: ("1") is not equal to ("2")
 Assertion Failure at VitalInfoSamplerTests.swift:93: XCTAssertEqual failed: ("1") is not equal to ("2")

@maxep maxep force-pushed the maxep/RUM-9521/change-rum-model-genetor-destination branch from a0cbd57 to 702e80b Compare April 24, 2025 12:47
@maxep maxep force-pushed the maxep/RUM-9521/change-rum-model-genetor-destination branch from 702e80b to 4ec0a0d Compare April 24, 2025 13:04
@maxep maxep force-pushed the maxep/RUM-9521/change-rum-model-genetor-destination branch from 4ec0a0d to f1a25e8 Compare April 24, 2025 13:21
simaoseica-dd
simaoseica-dd previously approved these changes Apr 24, 2025
View reviewed changes
mariedm
mariedm reviewed Apr 25, 2025
View reviewed changes
Copy link
Member

@mariedm mariedm left a comment

Choose a reason for hiding this comment

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

I just left a small question, otherwise LGMT!

TestUtilities/Sources/Mocks/DatadogInternal/RUMDataModelMocks.swift
session: .init(
hasReplay: hasReplay,
id: sessionID.toRUMDataFormat,
id: sessionID.uuidString.lowercased(),
Copy link
Member

@mariedm mariedm Apr 25, 2025

Choose a reason for hiding this comment

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

This might be due to resolving conflicts, but I believe @simaoseica-dd made these changes in #2265, so we should use RUMUUID and toRUMDataFormat now?

Copy link
Member Author

@maxep maxep Apr 28, 2025

Choose a reason for hiding this comment

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

Yes indeed, but that involves a @testable import DatadogRUM which I'm not willing to do in TestUtilities module. We should avoid such import in a non-test target.

Base automatically changed from maxep/RUM-9525/generate-model-initialisers to develop April 28, 2025 10:46
@dd-mergequeue dd-mergequeue bot dismissed simaoseica-dd’s stale review April 28, 2025 10:46

The base branch was changed.

@maxep maxep requested review from mariedm and simaoseica-dd April 28, 2025 11:41
@maxep
Copy link
Member Author

maxep commented Apr 28, 2025

/merge

@dd-devflow
Copy link

dd-devflow bot commented Apr 28, 2025 •
edited
Loading

View all feedbacks in Devflow UI.

2025-04-28 12:11:24 UTC ℹ️ Start processing command /merge

2025-04-28 12:11:29 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in develop is approximately 37m (p90).

2025-04-28 12:52:19 UTC ℹ️ MergeQueue: This merge request was merged

@dd-mergequeue dd-mergequeue bot merged commit ef7a966 into develop Apr 28, 2025
16 checks passed
@dd-mergequeue dd-mergequeue bot deleted the maxep/RUM-9521/change-rum-model-genetor-destination branch April 28, 2025 12:52
@maxep maxep mentioned this pull request May 20, 2025
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mariedm mariedm mariedm approved these changes

@simaoseica-dd simaoseica-dd Awaiting requested review from simaoseica-dd

Assignees

No one assigned

Labels

mergequeue-status: done

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@maxep @mariedm @simaoseica-dd