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

[API Proposal]: Track Exceptions in flight (information about the state that existed for the thread when the exception occurred) #98878

Closed
Yun-Ting opened this issue Feb 23, 2024 · 6 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-NativeAOT-coreclr needs-author-action An issue or pull request that requires more info or actions from the author.

Comments

@Yun-Ting
Copy link

Background and motivation

Currently, Marshal.GetExceptionPointers is not supported in native AOT.

See the environment control flag in runtime:

public static bool IsMarshalGetExceptionPointersSupported => !IsMonoRuntime && !IsNativeAot;

See open-telemetry/opentelemetry-dotnet#5374 (comment) on how this could be achieved.

API Proposal

Currently, getting in-flight exception is not supported in native AOT:

FCIMPL0(EXCEPTION_POINTERS*, ExceptionNative::GetExceptionPointers)

I propose to enable this feature in native AOT:

FCIMPL0(EXCEPTION_POINTERS*, ExceptionNative::GetExceptionPointers)
{
    FCALL_CONTRACT;

    EXCEPTION_POINTERS* retVal = NULL;

    Thread *pThread = GetThread();
    if (pThread->IsExceptionInProgress())
    {
        retVal = pThread->GetExceptionState()->GetExceptionPointers();
    }

    return retVal;
}
FCIMPLEND

API Usage

See this issue: open-telemetry/opentelemetry-dotnet#5358

Alternative Designs

No response

Risks

No response

@Yun-Ting Yun-Ting added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Feb 23, 2024
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Feb 23, 2024
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Feb 23, 2024
@ghost
Copy link

ghost commented Feb 23, 2024

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

Currently, Marshal.GetExceptionPointers is not supported in native AOT.

See the environment control flag in runtime:

public static bool IsMarshalGetExceptionPointersSupported => !IsMonoRuntime && !IsNativeAot;

See open-telemetry/opentelemetry-dotnet#5374 (comment) on how this could be achieved.

API Proposal

Currently, getting in-flight exception is not supported in native AOT:

FCIMPL0(EXCEPTION_POINTERS*, ExceptionNative::GetExceptionPointers)

I propose to enable this feature in native AOT:

FCIMPL0(EXCEPTION_POINTERS*, ExceptionNative::GetExceptionPointers)
{
    FCALL_CONTRACT;

    EXCEPTION_POINTERS* retVal = NULL;

    Thread *pThread = GetThread();
    if (pThread->IsExceptionInProgress())
    {
        retVal = pThread->GetExceptionState()->GetExceptionPointers();
    }

    return retVal;
}
FCIMPLEND

API Usage

See this issue: open-telemetry/opentelemetry-dotnet#5358

Alternative Designs

No response

Risks

No response

Author: Yun-Ting
Assignees: -
Labels:

api-suggestion, untriaged, area-NativeAOT-coreclr, needs-area-label

Milestone: -

@jkotas
Copy link
Member

jkotas commented Feb 24, 2024

Currently, getting in-flight exception is not supported in native AOT:

The Marshal.GetExceptionPointers API is not the right API to use for your scenario. It is both unnecessarily compilated API to handle your scenario and also not 100% reliable for your scenario as pointed out in open-telemetry/opentelemetry-dotnet#5374 (comment) and our offline discussion.

Could you please describe what you would like to actually achieve in your scenario? We can then work on the design on the right API that is both reliable and not unnecessarily compilated.

@jkotas jkotas changed the title [API Proposal]: Track Exceptions in flight (information about the state that existed for the thread when the exception occurred) in native AOT. [API Proposal]: Track Exceptions in flight (information about the state that existed for the thread when the exception occurred) Feb 24, 2024
@vcsjones vcsjones removed the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Feb 24, 2024
@Yun-Ting
Copy link
Author

Could you please describe what you would like to actually achieve in your scenario? We can then work on the design on the right API that is both reliable and not unnecessarily compilated.

Hi @jkotas, automatically detect exceptions and set Activity status is what we want to achieve.
See more details here: open-telemetry/opentelemetry-dotnet#1853
Thank you.

@jkotas
Copy link
Member

jkotas commented Feb 27, 2024

I am not able to figure out the desired expected behavior from this issue and from the code. (I know what your current code does, but I do not think that it is the behavior you actually want.)

Could you please describe the desired behavior? When is ExceptionProcessor expected to call SetStatus(ActivityStatusCode.Error) on the activity?

@agocke agocke added the needs-author-action An issue or pull request that requires more info or actions from the author. label May 13, 2024
@MichalStrehovsky
Copy link
Member

@reyang or @CodeBlanch could you give us some details on what semantics are needed for tracking exception in flight in OpenTelemetry? This is in reference to open-telemetry/opentelemetry-dotnet#1858. We see Marshal.GetExceptionPointers is used and numerical values it returns are being compared. We don't document the numerical value, only the structure that is pointed by the pointer. It's not clear to us what comparing numerical values is achieving.

@agocke agocke added this to the Future milestone May 17, 2024
@jeffschwMSFT jeffschwMSFT removed the untriaged New issue has not been triaged by the area owner label May 22, 2024
@dotnet-policy-service dotnet-policy-service bot removed this from the Future milestone Jun 20, 2024
@jkotas
Copy link
Member

jkotas commented Jun 24, 2024

Duplicate of #89724

@jkotas jkotas marked this as a duplicate of #89724 Jun 24, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jul 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-NativeAOT-coreclr needs-author-action An issue or pull request that requires more info or actions from the author.
Projects
Archived in project
Development

No branches or pull requests

7 participants