Skip to content
This repository has been archived by the owner on Jun 10, 2020. It is now read-only.

Regression issue: appId is never set in response headers #956

Closed
lmolkova opened this issue Aug 13, 2019 · 2 comments
Closed

Regression issue: appId is never set in response headers #956

lmolkova opened this issue Aug 13, 2019 · 2 comments
Assignees
Labels
Milestone

Comments

@lmolkova
Copy link
Member

lmolkova commented Aug 13, 2019

Repro Steps

  1. create a new aspnet core app from the template
  2. send some requests

Actual Behavior

app always returns Request-Context: appId= (empty) header

Expected Behavior

non-empty appid is returned

Version Info

It was introduced in 2.8.0-beta1 with perf fixes:

In the below code lastIKeyLookedUp is always null and is never equal to requestTelemetry.Context.InstrumentationKey so lastAppIdUsed never gets a chance to be anything except null.

if (this.lastIKeyLookedUp != requestTelemetry.Context.InstrumentationKey)
{
this.lastIKeyLookedUp = requestTelemetry.Context.InstrumentationKey;
this.applicationIdProvider?.TryGetApplicationId(requestTelemetry.Context.InstrumentationKey, out this.lastAppIdUsed);
}
HttpHeadersUtilities.SetRequestContextKeyValue(responseHeaders,
RequestResponseHeaders.RequestContextTargetKey, this.lastAppIdUsed);

@lmolkova lmolkova added this to the 2.8.0 milestone Aug 13, 2019
@lmolkova lmolkova self-assigned this Aug 26, 2019
@cijothomas
Copy link
Contributor

when lastIKeyLookedUp does not match the current items ikey, we trigger a app id lookup, and stores ikey to lastIKeyLookedUp. I believe the issue is relying incorrectly on ApplicationIdProvider to resolve ikey to appid in the very 1st call. its not guaranteed to return app id for the 1st time (because it don't wait for task to complete), but gets it resolved by subsequent calls. The above perf fix prevented asking for appid once lastIKeyLookedUp is set.

This is an issue with actual AppIdProvider, not with a DictionaryProvider. Functional Tests used DictionaryProvider, hence did not catch the issue.

The fix would be to do look up when lastAppIdUsed is null.

@cijothomas
Copy link
Contributor

if (this.lastIKeyLookedUp != requestTelemetry.Context.InstrumentationKey)
                    {
                        var appIdResolved = this.applicationIdProvider?.TryGetApplicationId(requestTelemetry.Context.InstrumentationKey, out this.lastAppIdUsed);
                        if (appIdResolved.HasValue && appIdResolved.Value)
                        {
                            this.lastIKeyLookedUp = requestTelemetry.Context.InstrumentationKey;
                        }
                    }

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants