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

[2.0] Adding the PHP version #562

Closed
Jean85 opened this issue Mar 14, 2018 · 5 comments
Closed

[2.0] Adding the PHP version #562

Jean85 opened this issue Mar 14, 2018 · 5 comments
Milestone

Comments

@Jean85
Copy link
Collaborator

Jean85 commented Mar 14, 2018

A PR was open under the Symfony integration repo (getsentry/sentry-symfony#118) to add the PHP version as a default tag (thanks @hjanuschka !).

I turned it down because I think that this should be done in the base client here. I investigated a bit, and @mitsuhiko told me that there's an already supported context to do that, which looks like this:

"contexts": {
    "runtime": {
        "version": "7.1",
        "name": "php"
    }
}

IMHO this should target the new client, what do you think?

@Jean85 Jean85 added this to the Release 2.0 milestone Mar 14, 2018
@ste93cry
Copy link
Collaborator

I think that this should be done in the base client here

I strongly agree. We could initialize the global runtime context with the data or initialize the runtime context of the event as the result will be the same. Alternately, (maybe the clearer option, but also maybe too much over engineered) we could create a specialized middleware

@Jean85
Copy link
Collaborator Author

Jean85 commented Mar 15, 2018

Ok so #564 already implemented this for 1.x. If we port the tests from there into the 2.x branch we are good! 👍

@ste93cry
Copy link
Collaborator

I was thinking, would it make sense to create a RuntimeContext class that validates that the only values that can be set are the one allowed by the SDK interface and in the constructor it initializes itself with some good defaults?

@Jean85
Copy link
Collaborator Author

Jean85 commented Mar 15, 2018

Yes, very good idea! 👍

@ste93cry
Copy link
Collaborator

Closing since this has been implemented in both versions 1.x and 2.x

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

3 participants