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

Fix custom backend provider logging on Linux #1848

Merged
merged 3 commits into from
May 25, 2021

Conversation

ConnorMcMahon
Copy link
Contributor

@ConnorMcMahon ConnorMcMahon commented May 25, 2021

If a custom durability provider uses its ETW event source before we
instantiate the Linux logging architecture (like Netherite's ETW event
for instantiating its orchestration service), then the
OnEventSourceCreated method will call before our constructor finishes
instantiating the event source name for the custom provider. This
happens due to the ordering of constructor calls in .NET.

To work around this, we temporarily store all event sources before our
constructor establishes the durability provider event source name so
that we can enable the correct provider when we do establish the provider name.

resolves #1796

Pull request checklist

  • My changes do not require documentation changes
    • Otherwise: Documentation PR is ready to merge and referenced in pending_docs.md
  • My changes should not be added to the release notes for the next release
    • Otherwise: I've added my notes to release_notes.md
  • My changes do not need to be backported to a previous version
    • Otherwise: Backport tracked by issue/PR #issue_or_pr
  • I have added all required tests (Unit tests, E2E tests)

If a custom durability provider uses its ETW event source before we
instantiate the Linux logging architecture (like Netherite's ETW event
for instantiating its orchestration service), then the
OnEventSourceCreated method will call before our constructor finishes
instantiating the event source name for the custom provider. This
happens due to the ordering of constructor calls in .NET.

To work around this, we temporarily store all event sources before our
constructor establishes the durability provider event source name so
that we can enable the correct provider when we do establish the provider name.

// Validate that the JSON has DurableTask-AzureStorage fields
string[] lines = consoleOutput.Split('\n');
var azureStorageLogLines = lines.Where(l => l.Contains("DurableTask-CustomSource") && l.StartsWith(prefix));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should change name of variable here.

Copy link
Contributor

@davidmrdavid davidmrdavid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this change! Seems like there are a few minor typos to address and I also left some comments.

Also, a question: Wouldn't it be possible to read the value for the durability provider from the app configuration itself inside of OnEventSourceCreated?
Say, would it be possible to read it as an environment variable when this.durabilityProviderEventSourceName is null?

I think that would be a cleaner approach.

test/FunctionsV2/CustomEtwDurabilityProviderFactory.cs Outdated Show resolved Hide resolved
Copy link
Contributor

@davidmrdavid davidmrdavid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM so long as CI goes 🟢

@ConnorMcMahon ConnorMcMahon merged commit ba56a82 into dev May 25, 2021
@ConnorMcMahon ConnorMcMahon deleted the FixEventListenerOrdering branch May 25, 2021 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix support for ETW tracing on linux and custom backends
3 participants