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

RUMM-486 Fix span encoding error on iOS versions prior to 13.0 #147

Merged

Conversation

ncreated
Copy link
Member

What and why?

📦 Prior to iOS13.0 the SDK was unable to send traces, as Span serialization was always failing. This PR fixes the behaviour on iOS11 and iOS12.

How?

As discussed in https://bugs.swift.org/browse/SR-6163, the JSONEncoder doesn't support primitive types encoding. This was fixed in iOS13.

We need primitive types encoding to make following part of Open Tracing interface compatible with Intake requirement: "all tags must be send as String values".

func setTag(key: String, value: Codable)

As we can't fix JSONEncoder, a workaround was added - the tag value is encoded as array and two surrounding bytes are removed from encoded Data. This is the fastest and most performant workaround that I found.

Other things done in this PR

This PR is part of RUMM-486, so I made all production-code unit tests green on iOS11.1, iOS12.4 and iOS13.5. I hope this will be enough to automate tests execution on all supported platforms (coming in next PR).

I've also discovered that URLSessionSwizzlingTests are failing prior to iOS13.0. As this is uncompleted and non-public feature, I just disabled this tests temporary, so we can deliver working beta faster. This should be fixed in "tracing autoinstrumentation" scope.

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

@ncreated ncreated added this to the next-version milestone Jun 18, 2020
@ncreated ncreated requested a review from a team as a code owner June 18, 2020 14:56
@ncreated ncreated self-assigned this Jun 18, 2020
@ncreated ncreated merged commit 3eb6de6 into master Jun 19, 2020
@ncreated ncreated deleted the ncreated/RUMM-486-run-tests-on-all-supported-iOS-versions branch June 19, 2020 09:35
@ncreated ncreated removed this from the next-version milestone Jun 19, 2020
acgh pushed a commit that referenced this pull request Feb 9, 2021
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