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

Document and commit to decision around edxapp middleware spans #778

Closed
3 tasks
robrap opened this issue Aug 21, 2024 · 6 comments
Closed
3 tasks

Document and commit to decision around edxapp middleware spans #778

robrap opened this issue Aug 21, 2024 · 6 comments

Comments

@robrap
Copy link
Contributor

robrap commented Aug 21, 2024

During the New Relic work, we temporarily disabled spans for middleware. @timmc-edx mentioned he'd like to leave it this way.

AC:

  • Create ADR for disabling (or enabling) middleware instrumentation in edxapp.
  • Update comments around disabling, or ticket re-enabling, depending on decision.
  • Communicate the decision to other teams.

Notes/Questions:

  • Middleware instrumentation is currently disabled in edxapp, and likely enabled in other services, so we can still screenshot the differences.
  • What do we lose without the middleware spans when working on or debugging middleware issues or errors?
  • What are the workarounds if we leave this disabled? Could it makes sense documenting temporarily enabling in an environment if we are working on a middleware issue?
@robrap
Copy link
Contributor Author

robrap commented Aug 21, 2024

@timmc-edx: This is another one that probably should be assigned to you, so you have the chance to make the case for leaving this disabled. Thoughts?

@timmc-edx
Copy link
Member

I don't know that it needs to be assigned to me—I don't have strong feelings around it, and I think anyone can make the decision. I'll just link this from 692 for now.

@robrap robrap assigned timmc-edx and unassigned timmc-edx Aug 22, 2024
@robrap
Copy link
Contributor Author

robrap commented Aug 22, 2024

Thanks @timmc-edx. I unassigned you. That said - I thought you had an opinion somewhere to leave this disabled. If you do have any thoughts, you can document them in a comment on this ticket.

@timmc-edx
Copy link
Member

@timmc-edx
Copy link
Member

Communicated in observability Slack channel: https://twou.slack.com/archives/C048L3Y9RDG/p1730233003973139

I'm not sure we need an ADR for this, with the wiki docs in place. If we don't, then this issue is probably complete.

@robrap
Copy link
Contributor Author

robrap commented Nov 7, 2024

Closing as complete. Thanks @timmc-edx,

@robrap robrap closed this as completed Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

2 participants