GA splunk-otel-ios

[dvernon-splunk]

Which GDI repository do you wish to GA?

splunk-otel-ios https://github.com/signalfx/splunk-otel-ios

Does the repository follow the latest tagged minor release in GDI specification?

• [x] Has an appropriate maintainers team.
• [x] Permissions set correctly.
• [x] Branch protection in place.
• [x] Dependencies appropriately locked down.
• [x] GitHub Applications set up per spec.
• [ ] Follows the configuration requirements, if appropriate.
• [x] Follows the semantic convention requirements, if appropriate.
• [x] Required Files in place.
  • [x] CHANGELOG.md
  • [x] CODE_OF_CONDUCT.md
  • [x] CONTRIBUTING.md
  • [x] .github/CODEOWNERS
  • [x] LICENSE
  • [x] README.md
  • [x] Build status badge
  • [x] Getting started
  • [x] Troubleshooting
  • [x] Link to official Splunk docs
  • [x] License information
  • [x] SECURITY.md
• [ ] Releases done to spec.
• [ ] Type specific requirements (remove what doesn't apply)
  • [ ] Data Collector
  • [ ] Documents all supported configuration parameters.
  • [ ] Documents sizing guidelines
  • [ ] [Instrumentation Library](https://github.com/signalfx/gdi-specification/blob/v1.6.0/specification/repository.md#instrumentation-libraries)
  • [ ] Documents all supported configuration parameters.
  • [ ] Documents how to configure manual instrumentation.
  • [ ] Documents how to configure log correlation.
  • [ ] Documents minimum supported version of each auto-instrumentation framework.
  • [ ] Real User Monitoring Library
  • [ ] Documents all supported configuration parameters.
  • [x] Documents how to configure manual instrumentation.
  • [x] Documents supported instrumentation.
  • [x] Documents supported platforms.

How long has the GDI repository been public?

Two years

Is the repository known to be used today?

yes

Is there a date by which this approval is needed?**

No

Additional context

None

[dvernon-splunk]

@pellared this repo is ready for review

[Kielek]

@dvernon-splunk, I have removed all checkboxes from the description + updated links to 1.6.0. I will go through verification shortly.

[Kielek]

• [x] Has an appropriate maintainers team.
• [x] Permissions set correctly. @signalfx/gdi-instrumentation-admins - set as admins @signalfx/gdi-ios-approvers - set as write

[Kielek]

Fixed by: https://github.com/signalfx/splunk-otel-ios/pull/174

• [x] CHANGELOG.md
• [x] CODE_OF_CONDUCT.md
• [x] CONTRIBUTING.md
• [x] .github/CODEOWNERS
• [x] LICENSE
• [x] SECURITY.md

[Kielek]

[x] GitHub Applications set up per spec.

It is technically fine, but repository is not using CodeCov.

[Kielek]

For

• [x] Dependencies appropriately locked down.

Dependencies

• MUST lock the versions of all build dependencies (e.g. libraries, binaries, scripts, docker images) or vendor them; EXCEPTION: tools that are available out-of-the-box on the CI runner
  • for sure it is no done done for action/checkout in https://github.com/signalfx/splunk-otel-ios/blob/a9e464d31bb72398e9953f2b9bf708f087f52d95/.github/workflows/ci.yml#L12 and runner itself (macos-latest) https://github.com/signalfx/splunk-otel-ios/blob/a9e464d31bb72398e9953f2b9bf708f087f52d95/.github/workflows/ci.yml#L9 - https://github.com/signalfx/splunk-otel-ios/pull/177
  • I am not sure how to verify swift dependencies, could you please show me an example defining exact dependency?
• MUST enable Dependabot alerts
  • MUST grant access to alerts for the approvers and maintainers teams
  • I have added approvers team here
  • MUST enable Dependabot security updates
• MUST configure Dependabot version updates

[dvernon-splunk]

We only have one dependency, Swifter, which is found in the Package.resolved file https://github.com/signalfx/splunk-otel-ios/blob/a9e464d31bb72398e9953f2b9bf708f087f52d95/SplunkRumWorkspace/SplunkRumWorkspace.xcworkspace/xcshareddata/swiftpm/Package.resolved#L6 It is locked to 1.5.0

[Kielek]

• [ ] Releases done to spec.

• MUST have a signature or a checksum with signature for all release artifacts

  • SHOULD use Splunk signing key
    • as I see, you do not have any artifacts, excepts source code zip, it should be fin

• MUST use signed tags

  • -s while createing tag. It is fine.

• MUST have a tag protection rule for the release tags (e.g. v*)

  • done

• MUST state version of OpenTelemetry repository built against if applicable

  • I do not see any upstream, should be fine

• MUST update all examples in the Splunk OpenTelemetry example repository that depends on the repository to use the latest release.

  • There is a ios application: https://github.com/signalfx/tracing-examples/tree/main/ios/Demo%20Boutique/Demo%20Boutique which seems to be not update/instrumented by this reposiroty. Please do it, and extend you RELEASING.md to handle it.

[Kielek]

Do you have a plan to GA also: https://github.com/signalfx/splunk-otel-ios-crashreporting ? If so, I think that separate GA request is needed.

[Kielek]

• [ ] Follows the configuration requirements, if appropriate.

Based on https://docs.splunk.com/observability/en/gdi/get-data-in/rum/ios/configure-rum-ios-instrumentation.html and https://github.com/signalfx/gdi-specification/blob/v1.6.0/specification/configuration.md#real-user-monitoring-libraries You should change a lot of configuration names:

• realm - it is fine
• rumAuth -> rumAccessToken
• beaconUrl -> beaconEndpoint
• globalAttributes seems fine
• environment -> deploymentEnvironment
• appName -> applicationName

There is a chance that I have missed something while code review, but additional things are required: Please confirm that it is fine/fix issues:

[1] Application name, authentication token and either realm or the beacon URL MUST be provided by the user. If any of these is missing, the RUM instrumentation library MUST fail to start.
[2] Systems that allow implementations to enforce the beaconEndpoint value is https (i.e. not Android) MUST do so. These implementations need to reject/fail to start if this condition is not meet. Implementations MAY offer an unrecommended allowInsecureBeacon option (default false) that turns off the check.
[3] If both realm and beaconEndpoint are set, a warning saying that realm will be ignored SHOULD be logged.

Based on https://github.com/signalfx/gdi-specification/pull/275 (will be part of 1.7.0 GDI) you should change also. Technically you can release based on 1.6.0 but, it will introduce much more issues that fixing it before 1.0.0.

• enableDiskCache -> enableDiskBuffering
• spanDiskCacheMaxSize -> limitDiskUsageMegabytes

Other

1. allowInsecureBeacon - consider to drop this - per Implementations MAY offer an unrecommended allowInsecureBeacon option (default false) that turns off the check.. Not a hard requierment
2. spanFilter required definition on GDI-spec https://github.com/signalfx/gdi-specification/issues/278
3. ignoreURLs seems to duplicates spanFilter functionality
4. sessionSamplingRatio required definiton on GDI spec https://github.com/signalfx/gdi-specification/issues/279
5. bspScheduleDelay Seems to be unique to ios.
6. slowRenderingDetectionEnabled and slowFrameDetectionThresholdMs and frozenFrameDetectionThresholdMs - required gdi spec definition https://github.com/signalfx/gdi-specification/issues/280 - keep in mind that android defines only 2 options (frozenFrameDetectionThresholdMs can be dropped?)
7. debug - required gdi-spec changes: https://github.com/signalfx/gdi-specification/issues/281
8. showVCInstrumentation, screenNameSpans, networkInstrumentation seems to be ios specific

[Kielek]

• [x] Follows the semantic convention requirements, if appropriate.

rum section seems to be fine/

[Kielek]

• [ ] Real User Monitoring Library
  • [ ] Documents all supported configuration parameters. - requires update when you copy issues from https://github.com/signalfx/gdi-specification/issues/255#issuecomment-1786677554
  • [x] Documents how to configure manual instrumentation.
  • [x] Documents supported instrumentation.
  • [x] Documents supported platforms.

[dvernon-splunk]

Do you have a plan to GA also: https://github.com/signalfx/splunk-otel-ios-crashreporting ? If so, I think that separate GA request is needed.

Eventually, but not at this time