Add support of egress provider to deliver data to a S3 storage

[Egyptmaster]

No description provided.

[dnfadmin]

All CLA requirements met.

[birojnayak]

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.❌ Egyptmaster sign nowYou have signed the CLA already but the status is still pending? Let us recheck it.

please make sure to sign CLA by .net foundation

[birojnayak]

I also didn't see any Unit test, I also didn't find any for AzureBlob storage... we need to check with project owner about that.... thanks for working on it...

[wiktork]

Adding some initial feedback

[birojnayak]

@Egyptmaster could you please sign the CLA first ? and do you have an ETA when you are going to incorporate the changes ? thank you !

[Egyptmaster]

@Egyptmaster could you please sign the CLA first ? and do you have an ETA when you are going to incorporate the changes ? thank you !
Unfortunatelly there is currently a running discussion with my company leads about signing the CLA, hope that they accepted it soon. Afterwards I hoped to find the time to adapt the code according the comments.

[ghost]

The 'no-recent-activity' label has been added to this pull request due to four weeks without any activity. If there is no activity in the next six weeks, this pull request will automatically be closed. You can learn more about our stale PR policy here: https://github.com/dotnet/dotnet-monitor/blob/main/CONTRIBUTING.md#stale-pr-policy

[birojnayak]

@Egyptmaster could you please sign the CLA first ? and do you have an ETA when you are going to incorporate the changes ? thank you !
Unfortunatelly there is currently a running discussion with my company leads about signing the CLA, hope that they accepted it soon. Afterwards I hoped to find the time to adapt the code according the comments.

@Egyptmaster any updates ? if you don't have time, let me know , I can incorporate the changes needed for S3 egress cloning your PR and publish a new PR..

[Egyptmaster]

Sorry, wasn't my intention to close the pull-request. I finally received the go from my employer and signed the CLA. I will try to take care about the comments to finally get the PR done.

[Egyptmaster]

@Egyptmaster could you please sign the CLA first ? and do you have an ETA when you are going to incorporate the changes ? thank you !
Unfortunatelly there is currently a running discussion with my company leads about signing the CLA, hope that they accepted it soon. Afterwards I hoped to find the time to adapt the code according the comments.

@Egyptmaster any updates ? if you don't have time, let me know , I can incorporate the changes needed for S3 egress cloning your PR and publish a new PR..

Hope that I catch all your comments. Thanks a lot.

[birojnayak]

Sorry, wasn't my intention to close the pull-request. I finally received the go from my employer and signed the CLA. I will try to take care about the comments to finally get the PR done.

no problem... thanks for working on it.. I have reviewed and posted some comments..

[birojnayak]

thanks for working on it.. almost there.. some minor comments to incorporate

[github-actions]

Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit

dotnet format

src/Tools/dotnet-monitor/Egress/S3/S3Storage.cs|40|
src/Tools/dotnet-monitor/Egress/S3/S3StorageEgressProvider.cs|40|

[jander-msft]

I'm approving since I don't have any more comments or required changes at this time. However, there are build failures across all build legs. Please pull in the latest from main, update the schema.json baseline, fix all of the format violations that were added as comments, and correct any code analysis errors that should be appearing as part of the build.

[jander-msft]

By the way, we are working on making egress providers to be extensions rather than built into dotnet-monitor. See #2500. You'll likely see the code refactored and possibly even moved to another repository (along with the Azure Blog egress provider).

This should not impact your PR so long as you fix up all remaining issues and get it approved and merged soon.

[Egyptmaster]

I'm approving since I don't have any more comments or required changes at this time. However, there are build failures across all build legs. Please pull in the latest from main, update the schema.json baseline, fix all of the format violations that were added as comments, and correct any code analysis errors that should be appearing as part of the build.

Well, is that really necessary to handle each compiler waring as an error here. That was changed in the meanwhile, right? I cannot remember to see these errors before. That start to becomre frustrating but I try to adapt the code...

[jander-msft]

Well, is that really necessary to handle each compiler waring as an error here. That was changed in the meanwhile, right? I cannot remember to see these errors before. That start to becomre frustrating but I try to adapt the code...

Logging warnings as errors has been in this repository from the start, long before this PR was opened. However, what was changed was updating the code analysis toolset and applicable analyzers in #2771 on October 25th. This change flagged many violations compared to the previous toolset which needed to be addressed and any pending PRs need to adapt to them.

[Egyptmaster]

So now I only have a flacky test failing because of unknown reason.

System.Threading.Tasks.TaskCanceledException : A task was canceled.

Any clue?

[Egyptmaster]

I see that the pull-request is approved! Thanks a lot. What comes next, who is merging it to the main?