Feat performance #145
Feat performance #145
Conversation
rommelfreddy
force-pushed
the
feat-performance
branch
2 times, most recently
from
b0050ce
to
eb756d0
Compare
rommelfreddy
force-pushed
the
feat-performance
branch
from
eb756d0
to
f8eac4e
Compare
rommelfreddy
force-pushed
the
feat-performance
branch
from
97573d4
to
2c696dd
Compare
rommelfreddy
force-pushed
the
feat-performance
branch
from
2c696dd
to
8995a2f
Compare
|
hey @indykoning could you give me a first response on this, please? I will resolve the conflicts if you are willing to proceed with the merge. |
|
He's currently with vacation, back next week |
| private ConfigInterface $config, | ||
| private array $excludePatterns = [] | ||
| ) { | ||
| $this->excludePatterns = array_merge([ |
There was a problem hiding this comment.
Could you add a comment here why these are excluded, so if we or others look back at this we can quickly see why
| ]); | ||
|
|
||
| // Start the transaction | ||
| $transaction = startTransaction($context); |
There was a problem hiding this comment.
Just to make sure. Starting a transaction here won't count towards your Sentry usage and limits right?
Since we'll be executing it every request
| } elseif ($state->getAreaCode() === 'graphql') { | ||
| $this->transaction->setOp('graphql'); | ||
| } else { | ||
| $this->transaction->setOp($state->getAreaCode()); | ||
| } |
There was a problem hiding this comment.
Was there anything special that needed to happen here? If not we can remove the elseif
| } elseif ($state->getAreaCode() === 'graphql') { | |
| $this->transaction->setOp('graphql'); | |
| } else { | |
| $this->transaction->setOp($state->getAreaCode()); | |
| } | |
| } else { | |
| $this->transaction->setOp($state->getAreaCode()); | |
| } |
| SentrySdk::getCurrentHub()->setSpan($transaction); | ||
| } | ||
|
|
||
| public function finishTransaction(ResponseInterface|int $statusCode): void |
There was a problem hiding this comment.
Should we maybe wrap this in a try/catch as well like captureException so issues when collecting performance metrics can't interfere with rendering the page to a customer
related to #106
fixes #60
i've continued the implementation of the performance tracking and also added the profiling.
Thank you to @barryvdh for doing the base stuff