Skip to content

RUM-419 Report Application Build Number#1465

Merged
maxep merged 4 commits into
developfrom
maxep/RUM-419/add-build-version
Sep 15, 2023
Merged

RUM-419 Report Application Build Number#1465
maxep merged 4 commits into
developfrom
maxep/RUM-419/add-build-version

Conversation

@maxep

@maxep maxep commented Sep 12, 2023

Copy link
Copy Markdown
Member

What and why?

Send build_number field as part of RUM and Logs.

fix #1026

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

Custom CI job configuration (optional)

  • Run unit tests
  • Run integration tests
  • Run smoke tests

@maxep
maxep requested a review from a team as a code owner September 12, 2023 17:23
@maxep maxep self-assigned this Sep 12, 2023
xgouchet
xgouchet previously approved these changes Sep 12, 2023
Comment on lines +348 to +366
let applicationBuildNumber = configuration.bundle.object(forInfoDictionaryKey: "CFBundleVersion") as? String
?? "0"

@ganeshnj ganeshnj Sep 13, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

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


I don't recall exactly (it has been long time since I published an app) but can this be different than App Store build number?

If yes, then I expect build number (as well as application version) part of configuration (plain strings).

So the final resolutions looks like

  1. try from configuration string value
  2. try from bundle
  3. null (either missing from the payload or explicitly marked as null json value.)

❓ why is this a required field?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The build_version is a required field, same for the version field. We have not exposed an application version parameter in the core config, and I believe it should be the same for the build number.

They are both corresponding to values in the app store, but since we are getting them from the Bundle with optional casting to String, we have a fallback value that won't be reached in production.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't recall exactly (it has been long time since I published an app) but can this be different than App Store build number?

I explained our rationale here. In a nutshell:

@maxep
maxep requested a review from ganeshnj September 13, 2023 09:19
ncreated
ncreated previously approved these changes Sep 14, 2023

@ncreated ncreated left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me 👍. Worth linking: #1026

@maxep
maxep force-pushed the maxep/RUM-419/add-build-version branch 2 times, most recently from 7c46b35 to 8bb2e53 Compare September 14, 2023 14:47
@maxep
maxep requested review from ncreated and xgouchet September 14, 2023 14:47
@datadog-datadog-prod-us1

datadog-datadog-prod-us1 Bot commented Sep 14, 2023

Copy link
Copy Markdown

Datadog Report

Branch report: maxep/RUM-419/add-build-version
Commit report: 87a9d8b

dd-sdk-ios: 0 Failed, 0 New Flaky, 158 Passed, 0 Skipped, 13m 36.15s Wall Time

@maxep
maxep force-pushed the maxep/RUM-419/add-build-version branch from f90a33b to 87a9d8b Compare September 15, 2023 08:45
@maxep maxep mentioned this pull request Sep 15, 2023
6 tasks

@maciejburda maciejburda left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@maxep
maxep merged commit 438280f into develop Sep 15, 2023
@maxep
maxep deleted the maxep/RUM-419/add-build-version branch September 15, 2023 10:02
@maxep maxep mentioned this pull request Sep 26, 2023
8 tasks
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.

Version / Build number together with App version

5 participants