change in this file needs to be reverted, OpenTelemetry is presented in the runtime configuration

reverted

tiny mistake here, I believe

fixed

Updated

should we have some verification added here to check that onViewTracked is called (same for RumSessionScopeTest)? Or you want to rely solely on the integration tests?

imo, M receive an event with correct view counts W track multiple views in the integration test can make sure that onViewTracked is called, otherwise it won't have the correct view count

cannot we have a situation when we stopped session A and started session B, but due to async nature of different pipelines in the SDK, we have this method called with error coming from session A by the time startMetric was called with Session B? not sure what is the probability of that though.

I am not sure the probability of this neither, but with this pull request, the error coming from Session A will belongs to Session B since startMetric has called. I am not sure if we have anyway to improve this, since endMetric of Session A has been called, the telemetry is sent out, the error will not be able to catch the train which has left.

In theory we can have a skew in the metric data we receive in such case, so maybe it is better to rollback this change and still pass sessionId to this method from the DatadogContext we have in the TelemetryEventHandler?

I have two questions over this:

1. even if we use session Id to track the error, the error count in session B can be correct, but in the session A will always be less than the correct one since the metric can be sent out, is that something we can tolerate?
2. I tried to use sdkCore.getFeature(Feature.RUM_FEATURE_NAME)?.withWriteContext that you suggested to retrieve the session id for the error, I got always NULL_UUID, is that normal?

1. Don't know honestly, depends on the further metric usage and if we can have falsy data because of that.
2. You don't need to use it directly, you can hook inside the lambda in the TelemetryEventHandler or you can use something like RumContext.fromFeatureContext(sdkCore.getDatadogContext().featuresContext[Feature.RUM_FEATURE_NAME]). NULL_UUID means that session is not yet created.

The change is reverted, now we rely on the session id for the error tracking again

justification: W methodCall() already implies we are doing method call

fixed

your test is saying that you are tracking multiple views, but here it is the same view: same key and same name. Yes, under the hood it will be indeed multiple views, but to make test clear it is better to use different keys and names:

good point, fixed

this may make your test green in case of the bug. Imagine that you expect attribute buzz to be under full path foo.bar.buzz, but there is something in the model that has also path foo.buzz. Then your test will be green even if foo.bar.buzz is missing.

Check internals of RumEventAssert as an example, you can add the necessary method there and rely on the full path validation.

you are right, it's fixed

this is quite dangerous, you can cause infinite recursion here if somewhere in the RumFeature.sendEvent call stack there is call for the logger metric as well.

I think it will be simpler here just to record the data received in the property of StubInternalLogger and then query it.

now I keep a lastMetric inside StubInternalLogger to store the telemetry event

suggestion: you can also use static import for RumEventAssert.assertThat here.

fixed

blocking: IMO this looks like a hack.

Ideally, I don't think it should be Any argument type here, and probably this shouldn't be inside RumEventAssert. RumEventAssert is used for the RUM events only (the ones which are going to be sent to the intake), which are RUM View, RUM Error, etc. events.

In your case you have argument which is not a RUM event (and obviously none of the RUM events can have "wasStopped" or "hasViewCount" attribute). This approach works, but it is mixing the things. Maybe it is better rather to create another assertion class (say for Map<String, Any> or even Map<*,*>) and since you are not operating with JSON you can write something like that there (let's assume actual is of type Map<String, Any>):

fun hasWasStopped(wasStopped: Boolean): TelemetryMetricAssert {
        assertThat(actual).containsPathEntry(path, value)
        return this
    }

and then you just need to implement containsPathEntry which will take the path like additionalProperties.rse.views_count.total in a similar manner how it was done for json (just split in the fragments and iterate over).

TelemetryMetricAssert class is now created for this, please check it

following approach of capturing all the data in the test, it is probably more useful to record all the events (and then access the necessary using last() or findLast, etc.), also making this property immutable

updated

to be precise, you need to use filter { it["type"] == "mobile_metric" } here, because property itself is called telemetryEventsWritten, so if someone else adds there non-metric telemetry, telemetryEventsWritten.last() will give a wrong result and things won't be correct.

Make sense, I use lastOrNull{ it["type"] == "mobile_metric"} to return the last telemetry metric recorded.

no need to be mutable reference.

fixed