Glimpse v2 - Instance based framework provide, code configuration, OWIN and more

[avanderhoorn]

Version 2 Tracker

This branch represents the sum of all work done on Glimpse v2. Any work from v2 should be branched from here and ultimately brought back in.

Here is a list of work items that need to be completed for v2 release.

Done and merged into the version-2 branch:

• [x] Infrastructure abstraction to help support more platforms (i.e. OWIN, NancyFX, etc) #266 @nikmd23
• [x] Global access to common constructs for plugin/extension authors (i.e. logger, message bus, etc) #266 @nikmd23
• [x] Finding end body tag under all conditions #555 @CGijbels
• [x] Message bus events across async task being picked up (becomes important for async ADO, etc) #709 @CGijbels
• [x] Global access to configuration for end users (i.e. to enable/disable glimpse/tabs, etc) #741 @nikmd23
• [x] Custom event API for registering general and/or timeline events #745 @nikmd23
• [x] Allows metadata for tabs to be extended #750 @avanderhoorn
• [x] Allow instance/request specific metadata 4222513 @avanderhoorn
• [x] Add PRG support back into the system 269b3d5 @avanderhoorn
• [x] Dynamic configuration sections within web.config #686 @CGijbels

Done and yet to be merged

*none currently*

Work in progress

• [ ] How does XML config live side by side with code config @CGijbels
• [ ] Improved serialization process to safely process more object types #457 @CGijbels
• [ ] Bundle registered client scripts #753 @MisterJames
• [ ] Review of persistent store interface #790 @nikmd23
• [ ] How do we correctly inject the client/script correctly #791 @CGijbels is finishing this up

Yet to be looked at

• [ ] Review of timeline/trace relationship/api
• [ ] How to deal with large objects/data payloads
• [ ] Sort out how messaging moving forward (extension methods, inheritance, etc)
• [ ] Templating stagey for client and server
• [ ] UI/UX refresh (involves - Review of existing plugins to ensure they are focused and on task, improve history semantics within client and review information architecture of plugins and the relationship between HUD and tabs)
• [ ] Tab "profiles" that allow different sets of tabs to be shown for different reasons (e.g. data access tabs for DBA's, all tabs for devs or web API tabs for web API handled requests and mvc tabs for MVC handled requests)
• [ ] Disk based persistence store. SqlLite or something similar?

Integration with 3rd parties

(acts as a sanity check for our Infrastructure abstraction implementation)

• [ ] Owin/Katana tabs and insights #739
• [ ] WebAPI tabs and insights #715
• [ ] NancyFX tabs and insights repo/branch
• [x] ~~Simple.Web tabs and insights~~ Simple.Web is no longer in development
• [x] ~~FubuMVC tabs and insights~~ FubuMVC is done

Core people driving this work are as follows: @avanderhoorn @nikmd23 @CGijbels @csainty @darrelmiller @thecodejunkie @jeremydmiller @markrendle @dahlbyk @PaulAtkins

Sorry in advance for all the code comments below. There is a fair amount of work and discussion that has done into this work, hence the length of this PRs content.

[avanderhoorn]

Most of the recent commits have been to do the following

• Todo's - Tried to address as many of the todo's around GlimpseRuntime as I could. Still a few to go but got a fair few of them
• Refactoring - Aimed to abstract out what code could be brought out of GlimpseRuntime and trying to make the code a little more SOLID. This makes the code easier to read and reuse, but mainly designed to be easier test. Will probably take another pass in the future as well. Most of this work resulted in MetadataProvider, InspectorProvider, TabProvider and DisplayProvider. If anyone has any feedback on this let me know.
• PRG Support - Took a good crack of reintroducing PRG support. This has been missing for the entirety of v1. I've tested that it works with MVC style action redirects, but I haven't yet looked into WebForms. In this later case I think it could be a little more complex as redirects can cause EndRequest not to fire. I know @CGijbels was looking into this.
• Instance/Request Metadata - Support is now in place to allow metadata to exist on a request by request basis. This was required for the PRG support mentioned above. The client takes this instance based metadata and merges it over the top of the static metadata.
• Cleanup - Since there has been a few hands involved in getting GlimpseRuntime to where it is for v2, I address any inconsistencies I found as I went. This resulted in some of the refactorings mentioned above.

[avanderhoorn]

Note, PRG support currently has a minor UI glitch if you use it in conjunction with inspectig a history/ajax request. I'm not fixing this for the moment since this UI will probably change with UI refresh yet to come.

[CGijbels]

@avanderhoorn For WebForms to make the PRG work (and for a lot of other issues as well), we need to change the definition of when the EndRequest on the GlimpseRuntime will be called. Because at the moment the EndRequest is called (in ASPNET context) on PostReleaseRequestState httpApplication event, which is not the correct one for a true end request. I already had a suggestion in #606 , which is not complete, but illustrates the problem domain. In short the httpApplication's EndRequest is the one you want, because that one is always raised for each request, independent of the request succeeding or failing (which is the case when a Response.Redirect is done while indicating that the response is complete). The problem can be that we will then be calling GetData on tabs, later in the event pipeline (after PostReleaseRequestState) and possibly even when other events have been skipped (in case of errors, ...). So this might actually be quite breaking for existing tabs if they assume a specific order (which they mostly do because they have no idea when they are called (GetData has no idea about the current runtimeevent it is called for)

[CGijbels]

I also merged the latest version of master with version-2 and solved some merge conflicts. Now that this is done, TeamCity is actually picking this PR back up for an automated build (which was missing lately)

[CGijbels]

The build succeeded in the meanwhile

Oh, as a side note, I already did the reverse integration with master, since we decided to rebase #686 on version-2 instead of master, and since the work was already rebased on master, the merge then could have less conflicts, although I already know some merge conflicts are coming my way due to the Factory being gone

Anyway, #686 will be brought in next...

[avanderhoorn]

@CGijbels Great work! Have to ponder more on the PRG stuff. If we need to break or change something, now would be the time, but have to look at the other issues.

[avanderhoorn]

Dynamic config - So given the freedom we have with v2, is there anything that we wanted to change with this work? I know we talked about wanting to support stuff that isn't discoverable dictionary based...

Also more generally speaking, we need to look at how the xml configuration works with the coded config. Specifically:

• How does it work if someone defines a path for where they want dlls to be discovered in XML and in code
• What does the api/interface look like to allow us to use code to customize the custom configs (I know we talked about this, but just wondering if that is in place yet)
• In the end can we have custom root config elements as well as custom sub elements for the root elements we ship out of the box

[CGijbels]

I've checked in a little breaking change regarding the framework providers.

• The GlimpseRuntime.IsInitialized is renamed to GlimpseRuntime.IsAvailable
• The GlimpseRuntime.Initialize(...) must be changed to GlimpseRuntime.Initializer.Initialize(...)

The GlimpseRuntime.Initializer also allows adding host initialization messages in case you want things to be logged into the Glimpse log, but the Glimpse logger is not yet available because you still need to create it (we have such a case for the AspNet framework provider). You can see them as delayed writes to the Glimpse log once it is available (which will be once the configuration has been provided and before the actual initialization of the Glimpse Runtime)

[avanderhoorn]

Totally makes sense and great addition!

I wonder if this could be also rigged to work to buffer messages are are sent prior to Glimpse init? I know this brings up a whole heap of issues of when do we show those pre messages as we have made things a bit more ridged around what classifies as being a request and app init is pre request. But worth thinking about.

[CGijbels]

@avanderhoorn I'm not quite following you when you say

I wonder if this could be also rigged to work to buffer messages are are sent prior to Glimpse init?

Because that is exactly what is now possible, just use the GlimpseRuntime.Initializer.AddInitializationMessage() method and those messages will be picked up once the Glimpse runtime is initialized. We could also say that the AddInitializationMessage() method writes directly to the Glimpse logger if initialization already occurred.

We could even bring it further, by adding a static LogMessage() on the GlimpseRuntime that forwards the call to the initializer in case the Glimpse runtime is not yet initialized or writes it directly to the logger... but maybe then the GlimpseRuntime.Instance.Configuration.Logger will not be used anymore?

Or what other scenario did you have in mind when you mentioned the above?

[avanderhoorn]

Oh ok... so we might be a bit mixed up ;) Are you talking about log messages or message broker messages or both?

[CGijbels]

@avanderhoorn I was talking about plain log messages. We had some code commented out in the HttpModule static constructor, because the Factory got removed, hence no option any longer to do logging pre Glimpse runtime initialization.

MessageBroker messages seem a lot rarer as you can't get a handle to the MessageBroker if the GlimpseRuntime.Instance is not available. I think, correct me if I'm wrong, that the case you're referring to, is the AdoInspector and related Support class who are actually being invoked before the initialization of the Glimpse runtime (during application init). There I would use those startup messages to add the additional data.

I don't see directly if it would make sense to actually delay deliver message broker messages, because which request should get it? Maybe the first one, but even that one might decide on EndRequest not to store any information at all... , or do you see other use cases?

[CGijbels]

I added another breaking change because I removed the GlimpseRuntime.ExecuteDefaultResource(), framework providers should now call the GlimpseRuntime.ExecuteResource(...) and just pass in the resource name if available and otherwise null. If it is null, then the GlimpseRuntime will set it with the default resource.

Besides the advantage of minimizing the API surface of the GlimpseRuntime the default resource can now also accept ResourceParameters which was not possible up until now.

The invokers (HttpHandler or GlimpseMiddleware or NancyMiddleware) still need to pass in the ResourceParameters and resource name, which we could avoid if we would adapt the IRequestResponseAdapter so that it would provide the querystring parameters as a KeyValuePair array, which could then be used by the GlimpseRuntime to determine the resourcename and parameters.

@avanderhoorn or @nikmd23 Is there a particular reason not to expose the Querystring as a KeyValuePair array on the adapter?

[avanderhoorn]

All makes sense. Here is some responses:

• Message broker: Ya, it starts getting complicated when looking at what to do with broker messages before our init happens.
• GlimpseRuntime changes: sounds great to me. I think this is a good improvement and the fact that it can have query string parameters probably isn't a bad thing.
• Adapter querystring: I think this just might have been an oversight. As far as I'm concerned it can b added in.

[grahammendick]

I saw some comments about PRG in Web Forms, but nothing for a quite a while. Just checking this is still on the radar for v2?

[CGijbels]

@grahammendick Yep, that is the idea, because the change needed for that is relative small (see this POC PR) and it would not only support the typical PRG pattern but also those cases where Response.End gets called (ScriptHandler ...) unrelated to PRG. The main reason why we did not yet finish this is the hesitation to introduce a breaking change for tab owners, as it would mean that the GetData will be called during an exceptional flow as well...

On the other hand we might consider making these calls when they don't expect it and if they would fail we can still log the exception and proceed with the next tab, resulting in at least some data being captured and stored, opposed to none today...

But keep in mind that for PRG there are 2 parts, first the capturing part, which exists for MVC and will be added for WebForms, and secondly support for this in the UI.

[grahammendick]

Yep, I'm using PRG as shorthand (I'm more interested in the Response.Redirect subset).

I guess you could make it backward compatible by collecting data twice for a single request - once at the request end to cater for PRG and second where you currently do in v1; and if you get to the second collection point you could clear out the data collected at the first point?

[avanderhoorn]

As far as I'm concerned, if we need to make a breaking change, now is the time to look at it. If possible lets recap exactly what needs to be done and what the break would mean for authors. Then we can make a call on what to do.

On 12 April 2014 15:08, grahammendick [email protected] wrote:

Yep, I'm using PRG as shorthand (I'm more interested in the Response.Redirect subset).

I guess you could make it backward compatible by collecting data twice for a single request - once at the request end to cater for PRG and second where you currently do in v1; and if you get to the second collection point you could clear out the data collected at the first point?

Reply to this email directly or view it on GitHubhttps://github.com/Glimpse/Glimpse/pull/738#issuecomment-40288869 .

[CGijbels]

@avanderhoorn I think @nikmd23 has a different view on that, from what I remember from our talks is that he wants to prevent as much as possible (if possible of course) introducing breaking changes for tabs as the last thing you want is having a lot of unusable existing plugins once v2 is released (from which we don't know if they are even being maintained).

On the other hand I do think we can introduce this as non breaking, but then we need to introduce an improved ITab aka ITab2 (bad name, but you get the idea) and check for those at runtime when we are handling a new situation.

@grahammendick the problem is not necessarily storing the data, but rather collecting the data. Currently the RuntimeEvent.EndRequest maps onto the ASP.NET httpContext.PostReleaseRequestState instead of the real httpContext.EndRequest and the latter is called whenever the request is almost finished, whether the request was ended normally or due to a premature ending like with a Response.End and that might have a subtle impact depending on the tab being interrogated for data. But as the POC PR demonstrates, we can skip the real EndRequest if the current EndRequest aka PostReleaseRequestState was raised as expected.

On the other hand we should not forget that we inject the Glimpse client in the outgoing response, and if we hook in at httpContext.EndRequest, then that might be too late as well. On the other hand, we can fix this if we decide to handle both events and in case we only get to handle the httpContext.EndRequest event, we only collect and store information, but we do not alter the outgoing response to inject our Glimpse client, which is fine in most cases as Response.Redirect or Response.End calls indicate that the normal flow should be terminated anyway and that the data can still be found through the history tab for instance.

So in the end I think we can remain backward compatible while adding PRG support and premature request processing endings, because we should not forget that this is an ASP.NET event pipeline kind of issue, which is not the case for other hosts like Nancy or Owin

[avanderhoorn]

@CGijbels Totally. When I said look at making a breaking change, its not quite what I meant. I agree with @nikmd23 about trying not to change ITab. But it doesn't mean that we can't introduce an extra interface that we look for that authors choose to opt into and for those who don't, we choose a sensible default.