Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Where have StackTrace and StackFrame gone? #14447

Closed
aggieben opened this issue Apr 16, 2015 · 59 comments
Closed

Where have StackTrace and StackFrame gone? #14447

aggieben opened this issue Apr 16, 2015 · 59 comments
Assignees
Milestone

Comments

@aggieben
Copy link

I'm working on a migration of an existing .NET project and for the most part have been able to find where things have gone in the CoreCLR landscape, but I've been having trouble with StackFrame and StackTrace. They can't be found on http://packagesearch.azurewebsites.net/, and indeed when I try to compile code that references those types, the compiler pukes an error message indicating that it doesn't know what those types are.

Where have those types gone?

@weshaggard
Copy link
Member

Can you share what scenario you are using them for? Depending on what you are using them for you can potentially use Exception.StackTrace or Environment.StackTrace instead. With that said we are working on trying to bring some form of these APIs to .NET Core.

cc @noahfalk

@aggieben
Copy link
Author

Well, in this particular case, I'm trying to get a version of MiniProfiler to work in ASP.NET 5 using CoreCLR (and I say "a version" because by the time I'm done with all the #if DNXCORE50 directives it's going to be a little different...). In MiniProfiler, StackTrace is used to show where time is spent in a webrequest, and each individual timing can display a partial stack trace which necessitates filtering by method names in each StackFrame.

Specifically:

var frames = new StackTrace().GetFrames();
if (frames == null || MiniProfiler.Settings.StackMaxLength <= 0)
{
    return "";
}

var methods = new List<string>();
int stackLength = -1; // Starts on -1 instead of zero to compensate for adding 1 first time

foreach (StackFrame t in frames)
{
    var method = t.GetMethod();

    // no need to continue up the chain
    if (method.Name == AspNetEntryPointMethodName)
        break;

    if (stackLength >= MiniProfiler.Settings.StackMaxLength)
        break;

    var assembly = method.Module.Assembly.GetName().Name;
    if (!ShouldExcludeType(method)
        && !MiniProfiler.Settings.AssembliesToExclude.Contains(assembly)
        && !MiniProfiler.Settings.MethodsToExclude.Contains(method.Name))
    {
        methods.Add(method.Name);
        stackLength += method.Name.Length + 1; // 1 added for spaces.
    }
}

return string.Join(" ", methods);

@noahfalk
Copy link
Member

@aggieben

Unfortunately this is one of the scenarios that the new StackTrace/StackFrame APIs still aren't going to support. For running in the windows environment we would encourage people to use profiling tools based on ETW for a few reasons:

  1. The overhead of ETW is lower
  2. ETW allows tracing both managed and native code
  3. ETW can be analyzed offline, which doesn't force symbolic information to be present on the client
  4. ETW can capture a lot of other interesting information in addition to markers, which makes it easier to scale from simple profiling to more sophisticated scenarios.
  5. ETW based scenarios also work on .Net Native, where reflection information provided by APIs like StackFrame.Method is not available.

We've also found that many customers were unknowingly using 'new StackTrace()' to write code that was extremely fragile. For example they might count the number of frames between a well-known method and their current call and then make flow control decisions based on that. Or they might change the behavior of a method based on the caller. Unfortunately StackTrace never abstracted them from the JIT's inlining decisions and these coding patterns are very difficult to support in a back-compatible manner. On the whole we felt the ecosystem was getting more compat pain than benefit from the API. For now we've chosen not to support 'new StackTrace()'. I do realize that simplified self-diagnosis tools such as this one are casualities of that choice.

I'd be very happy to get your feedback or discuss it with you more.
HTH,
-Noah

@aggieben
Copy link
Author

I don't think ETW is a good fit for what I'm trying to do here: the whole point is to not need profiling tools at all, at least not to start with. This is especially so with CoreCLR where cloud and containers are the primary deployment targets, and access to more robust profiling tools may be cumbersome to obtain, or simply not available. Cross-plat is also a concern.

I'm not terribly interested in new StackTrace() per se; I just want to get a collection of the current stack frames and I don't really care where from. If that's not going to be supported in a new StackTrace API, then what is?

@mattwarren
Copy link
Contributor

I don't think ETW is a good fit for what I'm trying to do here: the whole point is to not need profiling tools at all, at least not to start with.

I was wondering the same thing, my understanding of ETW is that it's great for looking at a process from the outside, but it seems a bit overkill for doing something like this, when you're already inside a process?

Also it you are removing new StackTrace() are you removing Environment.StackTrace as well?

Finally, what's to stop someone throwing an exception and then catching it (straight away), accessing the Exception.StackTrace and then parsing the Stack Frames that way? (Or will this be removed as well?)

@noahfalk
Copy link
Member

The portion of StackTrace API we are enabling now only has the constructor for StackTrace(Exception, bool), which can give you programmatic access to the same data you would get from Exception.StackTrace.
You certainly could throw and catch an Exception immediately, but the stack trace on an exception only contains the frames it has unwound through so far. Thus you would only get one frame, the current method that just executed your try/catch.

We aren't disabling Environment.StackTrace, and if you wanted to log the stack trace strings that should work just fine. We were anticipating that developers would recognize that string parsing was more risky from a compat perspective than calling an API, and that this would discourage folks from taking a dependency on the contents of Environment.StackTrace for control flow decisions. The code example above is perhaps in the grey area. Imagine if a newer version of the JIT started inlining AspNet entry point method into its caller or the ASP net team changed the name of the method at some point. The Miniprofiler would start returning callstacks that went all the way back to thread start. That probably isn't horrible, but it is an example of how this style of code can take dependencies that aren't resilient across versions. On .Net Native Miniprofiler would trigger NullReferenceException because StackFrame.Method will return null.

It makes sense that if you want cross platform, ETW as it exists today, isn't the best fit for you. We are interested in figuring out what cross platform solutions look like, but haven't gotten there yet. Regardless we can ponder what might work better here. I'd like to help tools like yours be successful while still discouraging people from past bad practices. In my mind a critical element of the past bad coding patterns is that they let a thread synchronously obtain information about its caller. If the data isn't available until later I suspect it would still be fairly reasonable for diagnostics but becomes much harder to use for non-diagnostics. For example imagine there was an API where threads could call:
SnapshotCurrentStackTrace()
and then on some other thread, after a short period of time a listener might get a callback:
CallStackSnapshotAvailable( stackData );

I certainly don't have all the answers at this point, and there is nothing ideal to implement miniprofiler today, but I can make some suggestions on what next steps to a solution might look like:

  1. A proposal for an API and an analysis of how that API assists diagnostic tools while not making it too easy to misuse with control flow decisions based on callstack. My suggestion above might be a good starting point but I'm sure other approaches exist.
  2. A proposal of what stack trace data is collected, and what mechanism is used to collect it. Issues here involve multiple instruction architectures, and differing availability of reflection and symbolic information depending on runtime and application environment.

HTH,
-Noah

@aggieben
Copy link
Author

In my mind a critical element of the past bad coding patterns is that they let a thread synchronously obtain information about its caller. If the data isn't available until later I suspect it would still be fairly reasonable for diagnostics but becomes much harder to use for non-diagnostics.

I'm not sure I see the problem with this. All kinds of reflection can be done synchronously, including the gathering of runtime information about the current thread, process, etc. Are we to understand that in the new .NET world, all features that could allow "bad coding patterns" will be removed? Wouldn't the .NET Framework become dramatically less useful if that were to be the case?

Why isn't the answer to this problem good documentation instead of just removing useful functionality?

@noahfalk
Copy link
Member

Its not a blanket rule, but rather a case by case judgment for this particular API. Based on the evidence we've seen so far, on balance, new StackTrace() caused more problems for our developer community than it solved. For the new managed surface area we are also trying to be cautious in what we add because once something is present its hard to ever remove it. I don't treat our current choice as the last word on the matter. The community is welcome to discuss here and let us know that 'new StackTrace()' was more useful than we realized. However I'm also optimistic that there are alternate ways to support the diagnostics scenarios that want to walk callstacks without the same pitfalls that 'new StackTrace()' had.

@FransBouma
Copy link
Contributor

Profiling .NET apps which target databases is what our ORM Profiler does, and we too use StackTrace, to obtain call stacks to methods which perform work which is timed. If StackTrace and StackFrame are gone, we can't offer that to our users when we port our interceptor library (which runs in the app and intercepts the calls to ADO.NET elements).

I'm sorry, but removing it because some poorly educated pseudo-developer uses new StackTrace() in their code and that turns out to be brittle and a poor choice is a silly argument: every statement/api can be misused and also have use cases which do work and are valid (like this one).

Please consider adding this, as applications can now not offer the same features on .NET core as on .NET full (e.g. I now can't record stacktraces of call origins to a slow query in asp.net on .net core)

@noahfalk
Copy link
Member

@FransBouma
Thanks for the feedback!
I fully agree that removing an API merely because it is possible to shoot yourself in the foot isn't the goal. The evaluation we aim for is cost/benefit... how likely is it that the API will make things worse vs. how likely is the API to be useful. Getting feedback that the API is useful in your scenario feeds directly into that equation, and helps balance the scales.

@aggieben as well...
To make the feedback a little more actionable, would you guys be interested in providing a little more info about your profilers, how many users use them, or any other metrics that you think would be useful to understand the impact these tools have in the community? I imagine its possible for only a few tools to have substantial impact if they are consumed by many other .Net projects.

Also if you know of other tools that would like to use the APIs, by all means call them out or encourage their owners/users to come comment on this issue.

Last question for now, what do you envision would happen if your scenarios were one day AOT compiled like .Net Native? Enabling reflection information to be removed is a major performance goal for AOT, but APIs like StackFrame.Method rely on it being there. Do think your users would prefer a substantial increase in size on disk to keep the tools functioning as they are today, or that they might prefer migrating to tools that can can look up the stack trace data in PDBs, potentially offline, if it lets them preserve smaller binaries.

Thanks!

@aggieben
Copy link
Author

@noahfalk I don't know about others, but MiniProfiler is widely used. It has a fairly high-traffic repository:

image

and is frequently downloaded from NuGet.org:

image

It is used by Stack Exchange, for example, as well as many others. The purpose is simple: get instrumentation from a web application per request without needing external tools. It collects timings for a given request, and if enabled, renders those results to the UI in a compact way, including the ability to expand a particular timing to see a partial callstack.

This is a very useful tool for basic profiling because it's granular and low-friction. This is particularly the case in production sytems where Visual Studio is unavailable, or remote profiling is impossible technically or by policy. It's the kind of tool, once discovered, users wonder what they ever did before they knew about it.

The rest:

what do you envision would happen if your scenarios were one day AOT compiled like .Net Native?

What would happen? I couldn't say, but I don't think it would be apocalyptic. I think. It would kinda suck, though. How would we get stack traces in exceptions? Having stack trace snippets in MiniProfiler is useful for exactly the same reasons.

Do think your users would prefer a substantial increase in size on disk to keep the tools functioning as they are today, or that they might prefer migrating to tools that can can look up the stack trace data in PDBs, potentially offline, if it lets them preserve smaller binaries?

What do you mean, a "substantial increase" to keep things "as they are today"? Is reflection info suddenly getting a lot more expensive than it ever was?

Looking up stack trace info in PDBs might be ok, but it has to be possible at runtime to be useful to MiniProfiler (and preferably not stupid slow).

Anyway, I can't speak for everyone, but I kinda don't give a rat's rear about the size of the binaries (I mean, are we talking about a few MiB difference? I saw a GIF on Facebook this morning that was bigger than that...). Can I save money on an Azure or AWS compute instance by using 50MiB less on the filesystem by having skinnier assemblies? News to me if so. If the difference in size is very large, then we need to have another conversation, but from where I sit the size of the binaries is relatively unimportant within some constraint of "reasonable".

@FransBouma
Copy link
Contributor

@FransBouma https://github.com/FransBouma Thanks for the feedback!
I fully agree that removing an API merely because it is possible to shoot yourself in the foot isn't
the goal. The evaluation we aim for is cost/benefit... how likely is it that the API will make things
worse vs. how likely is the API to be useful. Getting feedback that the API is useful in your scenario
feeds directly into that equation, and helps balance the scales.

@aggieben https://github.com/aggieben as well...
To make the feedback a little more actionable, would you guys be interested in providing a little more
info about your profilers, how many users use them, or any other metrics that you think would be
useful to understand the impact these tools have in the community? I imagine its possible for only a
few tools to have substantial impact if they are consumed by many other .Net projects.

Our system is a commercial product so it's not used by a massive amount of users, maybe a couple of hundred. It's however a crucial aspect of our application to have a stacktrace to the calling code for a given query: with that call stack the user can determine where the slow query originates and can fix it more easily. Without the stacktrace this is a needle/haystack problem. So if this api is gone, our application and likely others which do the same, loses a lot of its value. 

Also if you know of other tools that would like to use the APIs, by all means call them out or
encourage them to come comment on this issue.

EFprof by @ayende likely also uses this Api and they too have a large group of users (likely more than us).

Last question for now, what do you envision would happen if your scenarios were one day AOT compiled
like .Net Native? Enabling reflection information to be removed is a major performance goal for AOT,
but APIs like StackFrame.Method rely on it being there. Do think your users would prefer a substantial
increase in size on disk to keep the tools functioning as they are today, or that they might prefer
migrating to tools that can can look up the stack trace data in PDBs, potentially offline, if it lets
them preserve smaller binaries.

Our users are server-side developers so it's not a concern for them what .NET Native will do so I can't answer that for you. The thing we as toolbuilders need is access to the call path to a given method at runtime. If there's another way to obtain that on .NET core (preferably not, as that would mean conditional compiles etc.) it's fine by me, as long as the call path is obtainable. If it's not, all kinds of profilers which intercept DB access are not usable. 

    Frans

@FransBouma
Copy link
Contributor

Reading @aggieben 's reply I agree with him regarding the .NET Native remark: size is of no concern, we're talking 'the future', not floppy disks, so I don't see why reflection data has to be stripped because it otherwise is too large. And if it is stripped because some low-memory device (an Amiga 500 Watch for example, running on 512KB memory, yes that's a joke) can't deal with the sizes, then our profiler can't be used on that device. I don't think that's a problem, these are developer tools: used primarily during development and if in production, on the server side.

@noahfalk
Copy link
Member

Appreciate it guys. For .Net Native its not that reflection info suddenly takes more space than it used to, but rather it isn't all turned on by default. When developers have to explicitly opt-in it makes it more obvious to them what perf costs of those features are. As a very rough estimate I usually assume half of any given MSIL image is metadata, and half of the metadata represents data that AOT would only need for reflection. I could guess that is single or low double digit MB for many apps but it really depends how much code customers are running.

In any case let me run some of this stuff down and chat with a few folks on the team who have more experience with developer pain part of the feedback on this API. Thanks!
-Noah

@ayende
Copy link
Contributor

ayende commented Apr 23, 2015

Note that stack traces are also crucial for things like generating tracing
& debugging in general.
Not to mention proper logs for production.

*Hibernating Rhinos Ltd *

Oren Eini* l CEO l *Mobile: + 972-52-548-6969

Office: +972-4-622-7811 *l *Fax: +972-153-4-622-7811

On Thu, Apr 23, 2015 at 1:19 AM, noahfalk notifications@github.com wrote:

Appreciate it guys. For .Net Native its not that reflection info suddenly
takes more space than it used to, but rather it isn't all turned on by
default. When developers have to explicitly opt-in it makes it more obvious
to them what perf costs of those features are. As a very rough estimate I
usually assume half of any given MSIL image is metadata, and half of the
metadata represents data that AOT would only need for reflection. I could
guess that is single or low double digit MB for many apps but it really
depends how much code customers are running.

In any case let me run some of this stuff down and chat with a few folks
on the team who have more experience with developer pain part of the
feedback on this API. Thanks!
-Noah


Reply to this email directly or view it on GitHub
https://github.com/dotnet/corefx/issues/1420#issuecomment-95354753.

@noahfalk
Copy link
Member

@ayende
Environment.StackTrace is already supported for scenarios where you simply need to print a textual stack trace to the console or a log. Does that cover scenarios you have in mind?

@HaloFour
Copy link

@noahfalk The logging infrastructure used by my company (large telephony ISV) absolutely depends on the ability to generate and walk the StackTrace in order to grab specific StackFrames and obtain the target MethodBase. Having to try to parse that from Environment.StackTrace sounds like an awful "work-around", particularly since we'd also have to manually determine depth from apparently a couple of frames within that call but then also attempt to resolve the strings to classes and methods. We've been excited by the prospect of considering CoreCLR on Linux as a supported platform but this would make that impossible.

@noahfalk
Copy link
Member

After taking a look at the comments here, checking back on some of the earlier data about the API causing trouble and doing a little research I think its fair to say we underestimated the value of the API. I no longer believe the trouble outweighs the value.

Someone will still need to think through some other aspects of the scenario and implementation, such as how this should behave on .Net Native (if it is part of System.Diagnostics.StackTrace contract they both share), if its worthwhile to make any alterations to discourage API misuse, if the performance is where it needs to be, and whether or not we can reuse some of the same machinery that generates Environment.StackTrace to implement this, etc. If someone in the community wants to run with this work right now let me know. Otherwise we'll leave this as an open issue. To set expectations up-front, I don't anticipate anyone from Microsoft is going to have time to handle this before the v1.0 release of .Net Core goes out the door, but it should be much easier to add functionality on an ongoing basis relative to the more monolithic desktop framework.

Feel free to chime in with any feedback... and thanks for your help in shaping .Net Core 👍

@noahfalk
Copy link
Member

@HaloFour
I get very scared when I hear you want to calculate stack depth using the StackTrace API or that you want to grab specific StackFrames. I can't be sure, but it sounds as though your logging system could fail if the JIT optimized away those frames? The StackTrace API provides no guarantee about which stack frames will be present. The results can change if you change versions of CLR or when the library versions change or the machine running the code changes or even two threads race against each other differently.

@HaloFour
Copy link

@noahfalk Skipping frames is only done for specific cases where an infrastructural piece logs on behalf of consuming code. In those cases we use the MethodImpl options to request no inlining. It's worked really well for us so far and it's been in use since .NET 2.0.

@aggieben
Copy link
Author

@noahfalk I'm interested in at least looking into it, although I dont' know much about .NET Native yet.

@noahfalk
Copy link
Member

@aggieben
I realized the System.Diagnostics.StackTrace.dll isn't in the open source portion yet (you've probably seen the long list of libraries on the corefx repo main page). I'm looking into what steps are involved to achieve that at my end.

For .NET Native I think someone from within Microsoft would need to assist but hopefully not all the work needs to be done in lock-step.

A first step is to take a stab defining precisely the scenarios that are going to be supported and the APIs that will be made available. I can tell you the existing work on System.Diagnostics.StackTrace.dll exposed this portion so far, with the goal of letting tools access stack traces from unhandled exceptions, either custom pretty-printing them in-process or sending the data to a remote server.

namespace System.Diagnostics {
  public sealed partial class StackFrame {
    internal StackFrame()
    public const int OFFSET_UNKNOWN = -1;
    public int GetFileColumnNumber()
    public int GetFileLineNumber()
    public string GetFileName()
    public int GetILOffset()
    public System.Reflection.MethodBase GetMethod()
    public override string ToString()
  }
  public static partial class StackFrameExtensions {
    public static System.IntPtr GetNativeImageBase(this System.Diagnostics.StackFrame sf)
    public static System.IntPtr GetNativeIP(this System.Diagnostics.StackFrame sf)
    public static bool HasILOffset(this System.Diagnostics.StackFrame sf)
    public static bool HasMethod(this System.Diagnostics.StackFrame sf)
    public static bool HasNativeImage(this System.Diagnostics.StackFrame sf)
    public static bool HasSource(this System.Diagnostics.StackFrame sf)
  }
  public sealed partial class StackTrace {
    public StackTrace(System.Exception e, bool fNeedFileInfo)
    public System.Diagnostics.StackFrame[] GetFrames()
    public override string ToString()
  }
}

@jgeurts
Copy link

jgeurts commented Apr 24, 2015

As a developer of .net applications, by not supporting these profiling applications with v1, I will have no desire to migrate existing projects or start new projects on v1.

Profiling code is a huge part of finding bottlenecks and retaining performance as a feature. Unfortunately, Microsoft's profiling tools are not nearly as user friendly or useful as the aforementioned libraries.

@noahfalk
Copy link
Member

@jgeurts
A very understandable position and I appreciate the feedback. We want to bootstrap the community as efficiently as possible, but naturally our resources within Microsoft are finite so we won't be able to unblock everyone on day 1. We hope getting a v1 out there increases community interest which accelerates feedback, adoption and community contributions. Ultimately I hope it means that projects like yours get more features faster than they would have, even if it can't all be in v1.0.

@haf
Copy link

haf commented Apr 25, 2015

I'm also using it for Logary and I know NLog is using it in code like this:

/// Gets the current name, as defined by the class-name + namespace that the logger is in.
[<CompiledName "GetCurrentLoggerName"; MethodImpl(MethodImplOptions.NoInlining); System.Diagnostics.Contracts.Pure>]
let getCurrentLoggerName () =
  let getName (meth : MethodBase, dt : Type) = match dt with null -> meth.Name | _ -> dt.FullName
  let sf, m = let sf' = StackFrame(toSkip, false) in sf', sf'.GetMethod()
  let cont (dt : Type) = not <| (dt = null) && dt.Module.Name.Equals("mscorlib.dll", StringComparison.OrdinalIgnoreCase)
  let frame_still_unrolling_or_is_in_dotnet_core = cont << snd

  Seq.unfold successor toSkip
  |> Seq.map (fun framesToSkip -> let m = StackFrame(framesToSkip, false).GetMethod() in m, m.DeclaringType)
  |> Seq.takeWhile frame_still_unrolling_or_is_in_dotnet_core
  |> Seq.append (Seq.singleton (m, m.DeclaringType)) // unless we found something, put the default in there
  |> Seq.last
  |> getName

@jakesays-old
Copy link

This is an example of yet another valuable feature being dropped. It is a trend that really has me worried, especially when features are removed because they are used incorrectly by some. Some of us know what we're doing, and don't appreciate having tools pulled at our expense.

I can understand prioritizing things to get to a V1, but to drop features to protect 'customers' from themselves at my expense is just wrong.

@HellBrick
Copy link

I encountered a weird use case for StackTrace class once: figuring out the logical entry assembly when Assembly.GetEntryAssembly() returned null. It sure was a dirty hack, but an incredibly useful one nevertheless.

@Suchiman
Copy link
Contributor

@noahfalk

Imagine if a newer version of the JIT started inlining AspNet entry point method into its caller or the ASP net team changed the name of the method at some point.

But wasn't it the point of .NET Core that you can choose which version you are ready to run on without affecting other applications on the machine?
So if i need this feature i can use it. if i want to upgrade .NET Core i can have a look at the changelog or at tests to see wether something bad happened and can either revert or fix it and deploy and only my application is affected.
Please don't cripple .NET Core into a PCL or java v2.

@MattWhilden
Copy link
Contributor

@JakeSays "It is a trend that really has me worried, especially when features are removed because they are used incorrectly by some."

Unfortunately, there's an even broader issue than just keeping people from hurting themselves.We'd also not like to build features in a way that make it harder to do work in other areas. As Noah points out above, being able to inspect the stack at runtime can lead to all manner of complicated issues in this regard... Two specific examples:

  • Inlining behavior across code generators. We want to make the code generators better but have seen a non trivial number of ugle compat issues.
  • Refactoring. We have had to revert refactorings because customer applications were taking a dependency on the method names in various stacks. Ugh.

That said, this perspective comes largely from the past decade of supporting the Desktop .Net Framework. You only have to have a Windows Update break Turbo Tax once to be mindful of your p's and q's for a while! :-) With the new vision of having runtimes/frameworks (and their updates!) application local, it's possible that we don't have to be as vigilant.

I completely sympathize with the frustration of valuable APIs missing in the new world but maybe we can come up with a pattern (or couple of patterns!) that allows us all to have the functionality we want without the headaches.

@noahfalk
Copy link
Member

@HaloFour and @HellBrick
Cool. Sorry to harp on that stuff over and over. If any future .Net developer searches their way to this thread I want to make sure the risks involved in using StackTrace are front and center for them ; )

I agree that language features such as the one you proposed seem like a more reliable solution for some of these scenarios.

@aggieben
Copy link
Author

@MattWhilden I think a talk like that would be awesome

@nexussays
Copy link
Member

@noahfalk @MattWhilden My mind is completely blown at these apocalyptic examples you guys keep throwing out. Who in their right mind would instrument their application based on StackFrame. It is for diagnostics as everyone here clearly understands and uses it for. Why are you throwing out these ridiculous scenarios as reasons to remove functionality?!

@MattWhilden
Copy link
Contributor

@nexussays I can understand your concern because it honestly does sound ridiculous. Sadly, these are absolutely not hypothetical. The ingenuity of a developer with Intellisense and a looming deadline never ceases to amaze! They take dependencies on all manner of "behavior" that you'd never expect anyone to. Sometimes by necessity, sometimes by accident.

I like to think about it as though these kinds of things were chainsaws. They're very useful in the right hands and dangerous in many others. Putting it in the garage next to all the other tools means that anyone in the household can find it and potentially hurt themselves. Maybe the house is better off without a chainsaw, maybe we can invent a better one that is less dangerous, but maybe we just have to leave it there and continue to deal with the ramifications.

With .Net Core we've tried to clean out the toolbox. Part of that is removing things that had a lingering support burden and seeing if perhaps we could live in a world without these types. As above, the evidence seems to be more strongly in favor than initially expected. We appreciate your feedback and consideration.

In the spirit of keeping this about the specifics of stackframe (implementation details, concerns moving to .Net Native etc), I've started an issue to discuss this at a meta level. Feel free to join me over there. 😄

@noahfalk
Copy link
Member

@nexussays
Its easy to miss in such a long issue, but did want to call out the current position on this is that I think we should have the functionality in spite of the way developers have misused it. Way up above I wrote:

After taking a look at the comments here, checking back on some of the earlier data about the API
causing trouble and doing a little research I think its fair to say we underestimated the value of the
API. I no longer believe the trouble outweighs the value.

The current issue is more one of getting time to work on it.

@nexussays
Copy link
Member

@MattWhilden @noahfalk Cheers, I can appreciate the horror upon seeing such things in the wild. But your chainsaw analogy is not accurate IMO.

A proper analogy would be car manufacturers discovering that people were hooking devices up to the diagnostics port on their cars and attempting to drive based on the feedback from the internal systems of the car. That is not a reason to get rid of the diagnostics port. Nor an example of how the diagnostics port is "dangerous".

Sometimes you just have to call a spade a spade; and in this case it's not a problem of chainsaws its a problem of idiots writing code.

@ayende
Copy link
Contributor

ayende commented May 19, 2015

As someone who is actively making use of this feature, I would strongly
disagree.
We need to have programmatic access to this for diagnostics, and that
doesn't mean just on the dev machine during debug. It can mean that we want
to run this in production to find out where the major costs are, for
example. For performance and error handling, or for quite a few reasons
that can generate really crucial behavior for the system.

*Hibernating Rhinos Ltd *

Oren Eini* l CEO l *Mobile: + 972-52-548-6969

Office: +972-4-622-7811 *l *Fax: +972-153-4-622-7811

On Tue, May 19, 2015 at 1:53 AM, Malachi Griffie notifications@github.com
wrote:

@MattWhilden https://github.com/MattWhilden @noahfalk
https://github.com/noahfalk Cheers, I can appreciate the horror upon
seeing such things in the wild. But your chainsaw analogy is not accurate
IMO.

A proper analogy would be car manufacturers discovering that people were
hooking devices up to the diagnostics port on their cars and attempting to
drive based on the feedback from the internal systems of the car. That is
not a reason to get rid of the diagnostics port. Nor an example of how the
diagnostics port is "dangerous".

Sometimes you just have to call a spade a spade; and in this case it's not
a problem of chainsaws its a problem of idiots writing code.


Reply to this email directly or view it on GitHub
https://github.com/dotnet/corefx/issues/1420#issuecomment-103240980.

@noahfalk
Copy link
Member

@ayende
Its easy to miss in such a long issue, but did want to call out the current position on this is that I think we should have the functionality in spite of the way developers have misused it. Way up above I wrote:

After taking a look at the comments here, checking back on some of the earlier data about the API
causing trouble and doing a little research I think its fair to say we underestimated the value of the
API. I no longer believe the trouble outweighs the value.

The current issue is more one of getting time to work on it.

@FransBouma
Copy link
Contributor

The current issue is more one of getting time to work on it.

So it now depends on whether a PM (who likely didn't participate in this thread) is convinced that this should be implemented?

@noahfalk
Copy link
Member

@FransBouma
Our team is rather PM light these days - I doubt any of them will weigh in on this ; ) The devs wind up making many of these calls. I mentioned above I doubt we'd have time to get to it prior to the initial release. However I'd be glad to have someone in the community work on it and aggieben already indicated interest.

@FransBouma
Copy link
Contributor

I know it's not your decision, but isn't it a bit cheap to leave essential infrastructure to volunteers so MS doesn't have to pay a developer to do it?

@richlander
Copy link
Member

@FransBouma There is no "cheapness" aspect in this. We've been taking the approach of making features up-for-grabs that we're currently not working on. There is no expectation that anyone will take us up on that. It's an option. Over on the CoreCLR repo, we've had great success working with the community on that.

Also, @noahfalk is being transparent about when he thinks the feature will be delivered. Folks have already convinced him that the feature is important to include. Turns out that he's a nice guy and listens to reason. Feel free to convince him that it's needed for v1. That might be the best avenue to get what you want, if you are not attracted to the up-for-grabs approach.

More generally, we had this idea that when we made the product open source on GitHub that some of the features would be developed by the community. This isn't some poorly thought out cost-cutting idea. We want to collaborate with .NET developers, deeply. No one but us could participate before in our closed source product. We've changed that. Our entire product is "essential infrastructure." I'm wondering if you disagree with the thesis of our open source endeavor? Certainly, some folks seem to quite like it.

Side point, I loved your car analogy, @nexussays. Very nice.

@eatdrinksleepcode
Copy link

Strongly agree with @richlander. We cannot (or at least we should not) praise Microsoft for not only releasing .NET as open source but actually running its future development as an open source project, and then turn around and attack them for asking the community to step up. This exact situation is the entire point of open source. If your priorities differ from those of the wider community or the maintainers, you can "scratch your own itch" by contributing yourself.

@nexussays
Copy link
Member

Since this issue was prefaced on determining the fate of StackFrame/Trace/etc and we have seemingly arrived at the conclusion that they should be (re)included in CoreFX. Can we close this issue and open a new one for the actual implementation (which references this issue and is marked "up for grabs")?

@richlander
Copy link
Member

@nexussays sounds like a fine idea. Do you want to open that issue? Once it exists and it references this issue, we can close this one. Certainly, we can mark it as up for grabs.

@nexussays
Copy link
Member

@richlander Sure, I can do that. I wasn't sure of the contribution policy around issues.

@richlander
Copy link
Member

@nexussays The contribution policy on issues is extremely liberal. Fire at will.

@nexussays
Copy link
Member

I don't seem to be able to add labels, but issue created.

@richlander
Copy link
Member

@nexussays Label added.

appeters referenced this issue in appeters/pact-net Mar 1, 2017
currently the StackTrace functionality in .net core is limited to a
string that needs to be parsed.  This method can be updated once the fix
is release:  https://github.com/dotnet/corefx/issues/1420
@NickCraver
Copy link
Member

@karelz The milestone label is incorrect here, this never made it into 1.0 - it's only available in 2.0+ AFAIK.

@karelz
Copy link
Member

karelz commented Mar 20, 2017

@NickCraver you're first person ever to care about milestones in closed issues (beside me ;-)).

I bulk edited ~4K closed bugs since December. Mostly just based on issue closed date as the best approximation.
This particular issue was resolved as dupe during 1.0.0 time-frame, however the main issue dotnet/corefx#1797 was fixed only for 2.0 as you point out.
I think it boils down to interpreting milestones on issues which do not track fixes in code - like dupes.
For example if I resolve today something as a dupe of a bug which is not yet fixed, but will be fixed (hopefully) in some Future release. Either I leave the milestone of the "dupe" empty and have to remember to change it one day when we fix the issue (complicated tracking, lots of effort). Or I just stamp it with current milestone, because it was resolved as dupe in current milestone time-frame (easy to do, somewhat trickier/confusing to reason about as this case shows).

I decided to follow the easier rules. It is significantly less work (I don't think anyone would fund the other option ever) and it can be reasoned about. The alternative (beside extra careful funding which is IMO sci-fi) is just chaos in milestones on closed issues ...
I am open to alternative view points ;)

vincentkam referenced this issue in vincentkam/mongo-csharp-driver Aug 29, 2018
Fix AggregateFluentTest's "Count_should_return_the_expected_result"
which was intermittently throwing a null pointer exception
 - Possibly related to https://github.com/dotnet/corefx/issues/1420
   - "The StackTrace API provides no guarantee about which stack
     frames will be present. The results can change if you change
     versions of CLR or when the library versions change or the
     machine running the code changes or even two threads race against
     each other differently"
vincentkam referenced this issue in vincentkam/mongo-csharp-driver Aug 30, 2018
Fix AggregateFluentTest's "Count_should_return_the_expected_result"
which was intermittently throwing a null pointer exception
 - Possibly related to https://github.com/dotnet/corefx/issues/1420
   - "The StackTrace API provides no guarantee about which stack
     frames will be present. The results can change if you change
     versions of CLR or when the library versions change or the
     machine running the code changes or even two threads race against
     each other differently"
@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 1.0.0-rtm milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Jan 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests