Rebrand Diagnostics Server

[josalem]

I'll continue to think about the naming. I'm not attached to the specific ones I suggested but I do worry that the term "reversed server" isn't currently planned to show up anywhere else in the overall experience or documentation. I know we have used the phrase internally, but if it shows up in the public facing name I anticipate customers won't find it intuitive. In order to understand 'reversed server' the customer would first need to know what the runtime diagnostic server is and that this term is a derivative from it. I don't think they will know either of those things.

One possibility is that because we haven't spoken much in public or in docs about "diagnostic server" there is probably room to change our planned terminology without causing too much confusion, though the window won't be open much longer. @josalem @shirhatti @wiktork @sywhang if you guys had thoughts?

One thought would be:

Runtime env var: DOTNET_MANAGEMENT_PORT=\foo\bar\mysocket
Tool: dotnet-monitor --dotnet-management-port=\foo\bar\mysocket

In both contexts I am making port refer by-default to the tool hosted port. If we did this I would stop talking about "Diagnostics Server" and instead say "the runtime has a management port" or "the runtime can connect to a tool's management port." We would also get rid of the current DiagnosticMonitorAddress* environment variables. We could support an optional more generalized syntax for DOTNET_MANAGEMENT_PORT allowing lists of ports, specifying if the runtime should connect or listen, and specifying if the runtime should suspend on start:
DOTNET_MANAGEMENT_PORT=\foo\bar\mysocket,connect,suspend;\baz\socket,listen,nosuspend

Originally posted by @noahfalk in dotnet/diagnostics#1303 (comment)

[Dotnet-GitSync-Bot]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

[josalem]

I like this idea. Essentially all the plumbing for multiple connections has already been done to enable the reverse scenario. This suggestion would simplify the semantics and make it easier to reason about for customers.

A couple sticky bits come with this though (we discussed some of these offline):

This change would allow different connections to request the suspend features. This would mean that resuming the runtime will become a "stateful" interaction since we would need to know which connection just sent the command and we couldn't resume until all necessary connections have resumed. This is definitely doable, but we'll need to plumb some state through to account for it.
Since this would be using an environment variable with a list, there's the possibility that customers could stomp out some latent configuration. We would need to add some guidance for tool developers and customers that they should set this variable like PATH and only append to it. This is a lot better than the current situation where there can only be one connection specified and if a customer sets it, they may be stomping a latent configuration.
Overall, I think this is all goodness and shouldn't require too much work (famous last words...). My one piece of feedback on this is the naming. Could we use a plural in the env var to hammer in the idea of a list and maybe use address instead of port? address feels a little less specific, while port makes me immediately thing of a TCP port.

Originally posted by @josalem in dotnet/diagnostics#1303 (comment)

[josalem]

Could we use a plural in the env var to hammer in the idea of a list and maybe use address instead of port? address feels a little less specific, while port makes me immediately thing of a TCP port.

The word "port" as a noun is used in other areas of computing beside TCP (especially for physical hardware). In general, it just means some endpoint from which you can access (or provide) information when connected.

If you add support, for example, for getting diagnostics information over TCP ports rather than pipes/domain sockets, would you use the same environment variables? Something like "DOTNET_MANAGEMENT_PORT=tcp:7000,listen,suspend"

What about "DOTNET_DIAGNOSTICS_PORTS" and "--diagnostics-ports" or if you do want to avoid "port" then "DOTNET_DIAGNOSTICS_ADDRESSES" and "--diagnostics-addresses"?

I don't think "management" is a good word to use here because you really don't manage anything with the connection (the runtime isn't being managed by this, the application isn't managed by this, etc; you could argue that one manages the diagnostics state of the runtime, but then it would be named something like DOTNET_DIAGNOSTICS_MANAGEMENT_PORT, and then we're getting into way too lengthy naming territory).

Originally posted by @jander-msft in dotnet/diagnostics#1303 (comment)

[josalem]

Something like "DOTNET_MANAGEMENT_PORT=tcp:7000,listen,suspend"

Yep!

What about "DOTNET_DIAGNOSTICS_PORTS" and "--diagnostics-ports"

This is fine by me. A couple nit questions to fully scout the naming:

1. Should it be DOTNET_DIAGNOSTIC_PORTS (removed the 'S' in DIAGNOSTICS)? I can't come up with technical reason why, but pronouncing the two words that end in 's' is harder. Is it just me?
2. I could imagine even if the runtime supports multiple ports that tools are under no obligation to do so. Anyone find it confusing if some places used the singular term 'diagnostic port' and others used the plural 'diagnostic ports'? (I don't)
3. Any concern that --diagnostic-port(s) would conflict with other argument names that dotnet-monitor would be likely to use? What are we calling the two dotnet-monitor endpoints where we are serving http traffic?

if you do want to avoid "port" then "DOTNET_DIAGNOSTICS_ADDRESSES" and "--diagnostics-addresses"?

Personally I prefer port, partly because it makes it a little easier to use "Diagnostic Port" like a proper noun and know what it refers to and partly because it doesn't clash with other service/proxy terminology that is likely to use 'endpoint' or 'address'.

Not sure if the syntax of the list was fully inferable from my examples but this is a little more fleshed out version of that part:

Ports := semicolon separated list of 'Port'
Port := address[,optional_tags]
optional_tags := comma separated list of 'tag'. suspend and nosuspend are mutually exclusive, as are listen and connect.
tag := suspend | nosuspend | listen | connect

When specified on the runtime env var 'suspend' and 'connect' are the defaults if not specified in the tags list. When specified in the tool argument 'listen' is the default and 'suspend'/'nosuspend' are ignored entirely if the user writes them. Does this syntax make sense? Its easy to change if there are alternate suggestions : )

Originally posted by @noahfalk in dotnet/diagnostics#1303 (comment)

[josalem]

I've come around on Port I think. It collates the idea of a communication mechanism and a "location" to access the mechanism succinctly and in a recognizable manner to most users.

To your points, Noah:

1. I think DOTNET_DIAGNOSTIC_PORTS sounds better as well, but I don't know why. Part of me wants to be consistent in how we name these things, but this is meant to rebrand the feature so we can do what we want right now 😄
2. I don't think it would be confusing. Though I do think we should strive to be consistent, I don't think it should be a sticking point.

I think your syntax works. I might go one step further and specify that the first tag observed overrules any subsequent tags for mutually exclusive tag sets. That ensures that we have a non-erroring behavior if someone accidentally over-tags their port.

I think most of the work to support multiple ports like this has already been done, so the remaining work in the runtime would be the following:

• Add DOTNET_DIAGNOSTIC_PORTS config knob
• Rename DOTNET_DIAGNOSTICS_MONITOR_PAUSEONSTART to something that matches this naming scheme specifically for the default port
• Add logic for consuming Noah's proposed syntax
• Add logic for multiple connections requesting PauseOnStart (probably the most time consuming part)

Let me know if I missed anything here. I don't see anything of particular risk. That being said, I don't think we have a test harness that could stress this right now, so that could be time consuming to build.

[noahfalk]

Rename DOTNET_DIAGNOSTICS_MONITOR_PAUSEONSTART to something that matches this naming scheme

Can we get rid of it? The PORTS syntax now includes a per-request setting to suspend so a global mechanism is probably superfuous.

[josalem]

I think you're right, but when I just thought about it, there is one thing we might want to leave it for: using the default port with pause on start. For example, let's say I want to implement something like dotnet debug my.dll. I could use that variable to turn on the pause on start functionality without needing to create an additional port and then I would just send the resume runtime command via the default port. Thoughts?

PS: this new terminology is making it much easier to discuss these concepts.

[noahfalk]

you are right, it could make sense to use that var (or a renamed var) to control the suspension on the default port

[jander-msft]

Not sure if the syntax of the list was fully inferable from my examples but this is a little more fleshed out version of that part:

Ports := semicolon separated list of 'Port'
Port := address[,optional_tags]
optional_tags := comma separated list of 'tag'. suspend and nosuspend are mutually exclusive, as are listen and connect.
tag := suspend | nosuspend | listen | connect

The specified syntax implies that when specified on the runtime that the address for the listen mode must be specified, which means that the server endpoint is configurable. As it is today, it is not configurable and is only found at \.\pipe\dotnet-diagnostic-{pid} or /tmp/dotnet-diagnostics-{pid}-{diambiguator}.

When specified on the runtime env var 'suspend' and 'connect' are the defaults if not specified in the tags list. When specified in the tool argument 'listen' is the default and 'suspend'/'nosuspend' are ignored entirely if the user writes them. Does this syntax make sense? Its easy to change if there are alternate suggestions : )

When specified on the runtime, shouldn't 'listen' be the default? Or is it that if the DOTNET_DIAGNOSTIC_PORTS env var is specified that it implies that connect mode is the default; the non-existence of this env var is 'listen' and 'nosuspend'?

I think that, given the behavior of of the diagnostic port without the env var, the existence of the env var should assume the same defaults unless otherwise specified. Thus, DOTNET_DIAGNOSTIC_PORTS='/path/to/port/file' would be 'listen' and 'nosuspend' by default (if specifying a socket file / named pipe path is allowable for runtimes in listen mode).

Here are my suggested defaults and explicitly laid out syntax combinations:

Runtime Port (Default Value)
/tmp/dotnet-diagnostics-{pid}-{diambiguator},listen,nosuspend

Runtime Port (Name: DOTNET_DIAGNOSTIC_PORTS)

Value	Implied	Comments
{path}	listen,nosuspend	If configuring port path in listen mode is allowed
{path},connect	nosuspend
{path},connect,nosuspend
{path},connect,suspend
{path},listen	nosuspend	If configuring port path in listen mode is allowed
{path},listen,nosuspend		If configuring port path in listen mode is allowed
{path},listen,suspend		If configuring port path in listen mode is allowed
{path},nosuspend	listen
{path},suspend	listen

Suggested 'PAUSEONSTART' Rename
Name: DOTNET_DIAGNOSTIC_PORTS_SUSPEND
Uses the same terminology as the rest of the env vars and values.
I would think setting this env var would override any of the suspend/nosuspend settings for each of the ports in the DOTNET_DIAGNOSTICS_PORTS env var.

Tool Argument Port (Default Value)
/tmp/dotnet-diagnostics-{pid}-{diambiguator},connect

Tool Argument Port (Name: --diagnostic-ports)

Value	Implied	Comments
{path}	connect	If configuring port path in listen mode on runtimes is allowed
{path},connect		If configuring port path in listen mode on runtimes is allowed
{path},listen

The 'suspend' and 'nosuspend' values are not applicable for the tool argument (either failure or just ignored).