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

Unified naming for span ops #1661

Merged
merged 15 commits into from Oct 10, 2022
Merged

Conversation

antonpirker
Copy link
Member

@antonpirker antonpirker commented Oct 5, 2022

Having a unified naming for span ops helps the performance team to discover performance issues.
The ops in the tests are all hard coded, so the tests show if someone trying to change ops string.

A list of all available span ops can be found in the develop docs:
https://develop.sentry.dev/sdk/performance/span-operations/

Fixes #1643

@antonpirker antonpirker changed the title Unified naming for span ops. Unified naming for span ops. (#1643) Oct 5, 2022
@antonpirker antonpirker changed the title Unified naming for span ops. (#1643) Unified naming for span ops. Oct 5, 2022
@antonpirker antonpirker marked this pull request as ready for review October 5, 2022 10:43
@sl0thentr0py
Copy link
Member

sl0thentr0py commented Oct 5, 2022

span op changes don't break anything but transaction op changes are breaking in terms of existing dashboards.
How do we want to push this out - major or minor with clear changelog entry?

@sl0thentr0py sl0thentr0py changed the title Unified naming for span ops. Unified naming for span ops Oct 5, 2022
@sl0thentr0py
Copy link
Member

also is the new consistent span ops table in notion or on develop already? If develop, can you please add the link to the PR description?

@antonpirker
Copy link
Member Author

Added link to develop doc to PR description.

Copy link
Member

@sl0thentr0py sl0thentr0py left a comment

Choose a reason for hiding this comment

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

changes look ok, pls don't merge till we decide on roll-out

@antonpirker
Copy link
Member Author

Note to self: document in the readme all the changes to span ops for everyone that has dashboards it could be breaking.

@antonpirker
Copy link
Member Author

Maybe we can find out how many dashboards would be affected with this change.

Possible ways to go:
a) document all changes in changelog in an easy understandable way
b) show a message in the sentry.io product if the user has the new python sdk installed and has dashboard to tell her that she needs to update the dashboards.
c) .. ? ..

@antonpirker
Copy link
Member Author

antonpirker commented Oct 5, 2022

The definite guide of changed span/transaction ops:

asgi.server -> http.server
aws.request -> http.client
aws.request.stream -> http.client.stream
celery.submit -> queue.submit.celery
celery.task -> queue.task.celery
django.middleware -> middleware.django
django.signals -> event.django
django.template.render -> template.render
django.view -> view.render
http -> http.client
redis -> db.redis
rq.task -> queue.task.rq
serverless.function -> function.aws
serverless.function -> function.gcp
starlette.middleware -> middleware.starlette

@antonpirker antonpirker marked this pull request as draft October 6, 2022 08:43
@antonpirker
Copy link
Member Author

Made a draft again so we do not accidentally merge it.

Also related to this one #1665

@antonpirker antonpirker marked this pull request as ready for review October 10, 2022 12:34
@antonpirker antonpirker merged commit c0ef3d0 into master Oct 10, 2022
@antonpirker antonpirker deleted the antonpirker/1643-unified-span-ops branch October 10, 2022 12:45
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.

Unified naming of span "op"s
2 participants