elastic / apm-agent-ios

Apache License 2.0
31 stars 19 forks source link

Provide configuration for exporting data using either HTTP or gRPC. #182

Open bryce-b opened 1 year ago

kylemay-gridstone commented 4 months ago

Hi, any ETA on this one? We cannot use gRPC and are currently having to use OpenTelemtry SDK directly which means we can't take advantage of some of the integrations in this library.

bryce-b commented 4 months ago

I can take a look at adding this for the next release.

kylemay-gridstone commented 1 month ago

Hi @bryce-b, AgentConfigBuilder doesn't use the value of connectionType. Raised https://github.com/elastic/apm-agent-ios/pull/235

kylemay-gridstone commented 1 month ago

There's also an issue at OpenTelemetryHelper:58 with a ( being added to the string at the start and failing to return a URL.

I could be wrong but it also looks like that is used without adding a URL path.

V-Zim-Connections commented 3 weeks ago

Following on previous comment you also will want to update: https://github.com/elastic/apm-agent-ios/blob/main/Sources/apm-agent-ios/OpenTelemetryInitializer.swift

https://github.com/elastic/apm-agent-ios/blob/b973118222c149180cfa12a46c1f8b5df55a6b7e/Sources/apm-agent-ios/OpenTelemetryInitializer.swift#L195

from

let metricExporter = {
let defaultExporter = OtlpHttpMetricExporter(endpoint: endpoint, config: otlpConfiguration, useSession: URLSession.shared)

to

let metricExporter = {
let metricEndpoint = URL(string: endpoint.absoluteString + "/v1/metrics")
      // TODO(team): Clean unwrap/check
      let defaultExporter = OtlpHttpMetricExporter(endpoint: metricEndpoint ?? endpoint, config: otlpConfiguration, useSession: URLSession.shared)

Replicate fix for traceExporter and logExporter

Of course having the paths hardcoded this way is not ideal and should be bubbled up in the PR but you get the idea, the paths are not configured correctly and would default to "/"

This has been tested and validated on stack 8.15.x

V-Zim-Connections commented 3 weeks ago

We will create the PR today @bryce-b