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

Azure storage logs should have a useful correlationId #1179

Open
jasonzio opened this issue Sep 28, 2022 · 3 comments
Open

Azure storage logs should have a useful correlationId #1179

jasonzio opened this issue Sep 28, 2022 · 3 comments

Comments

@jasonzio
Copy link

jasonzio commented Sep 28, 2022

Common practice is to create a correlationId for any user-facing service request and apply that single correlationId to all services invoked while handling that request. The django-guid plug-in is the standard mechanism for setting such a guid, and logs emitted by django itself include that id.

Azure storage API calls can optionally pass a correlationId which is attached to log entries created by Azure while handling the request. The guid passed into django-guid should be plumbed through to any calls to Azure storage APIs to enable log correlation across all services when investigating problems.

@jasonzio
Copy link
Author

I've submitted #1178 to address this.

@jasonzio jasonzio changed the title Azure storage logs don't have a useful correlationId Azure storage logs should have a useful correlationId Sep 28, 2022
sdherr added a commit to sdherr/django-storages that referenced this issue Oct 30, 2024
This PR changes no behavior except to provide a settings "hook" that you
can use to pass-through azure cli request options. There was already one
such option being passed through, `timeout`, so I've refactored things
slighly to avoid duplication.
sdherr added a commit to sdherr/django-storages that referenced this issue Oct 30, 2024
This PR adds a settings "hook" that you can use to pass-through azure
cli request options, and uses it to set the "client_request_id" by
default if django-guid is installed. There was already one such option
being passed through, `timeout`, so I've refactored things slighly to
avoid duplication.
@sdherr
Copy link

sdherr commented Oct 30, 2024

@jschneier In the interest of helping this Issue along I have created a couple of other pull requests, either of which would close this issue. You said here that your issues with the original PR were:

  1. You don't know django-guid and don't want to bless it.
  2. You didn't like the lack of obvious docs on the azure side.
  3. You didn't want to be stuck maintaining something that might decay in either direction without a good ability to test it.

In the first option, #1467, I provide a completely generic AZURE_REQUEST_OPTIONS setting "hook" that users can set to pass through any of the per-operation keyword arguments (some documentation does exist, it's just hard to find). This is extremely similar to the AZURE_CLIENT_OPTIONS setting which is already merged, except that these options are passed through to the requests, not on client creation, and additionally you can set an option with a callable in order to have it be evaluated at request time.

The second option, #1468, is identical except it by default sets client_request_id=django_guid.get_guid if django-guid is installed. I don't have any numbers on django-guid usage, however it has been around for a while and I know that at least the Pulp Project uses it to keep logs correlated, which is why we care.

I think that "use django-guid by default if installed" is the best level of dependency here to do the "right thing" from a user perspective, even if they don't know it. But if you disagree then hopefully the first option is acceptable; it allows users to set available Azure Storage options and would allow us to get back onto a standard version of django-storages (when merged / released).

I'll also point out that Microsoft provides free Azure credits for open-source developers to aid with your testing / developing, which you may want to look into regardless of these PRs. Please let me know if there are any changes you need in order to make this mergable.

sdherr added a commit to sdherr/django-storages that referenced this issue Oct 30, 2024
This PR adds a settings "hook" that you can use to pass-through azure
cli request options, and uses it to set the "client_request_id" by
default if django-guid is installed. There was already one such option
being passed through, `timeout`, so I've refactored things slighly to
avoid duplication.
sdherr added a commit to sdherr/django-storages that referenced this issue Oct 30, 2024
This PR changes no behavior except to provide a settings "hook" that you
can use to pass-through azure cli request options. There was already one
such option being passed through, `timeout`, so I've refactored things
slighly to avoid duplication.
sdherr added a commit to sdherr/django-storages that referenced this issue Oct 30, 2024
This PR changes no behavior except to provide a settings "hook" that you
can use to pass-through azure cli request options. There was already one
such option being passed through, `timeout`, so I've refactored things
slighly to avoid duplication.
@sdherr
Copy link

sdherr commented Nov 15, 2024

@jschneier ping, just a reminder that we are still interested in this. If there are any changes you want in order to make one of these PRs mergable please let me know.

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

No branches or pull requests

2 participants