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

Requests don't get logged into Xray #3

Closed
Sh1d0w opened this issue Jun 16, 2020 · 6 comments
Closed

Requests don't get logged into Xray #3

Sh1d0w opened this issue Jun 16, 2020 · 6 comments

Comments

@Sh1d0w
Copy link

Sh1d0w commented Jun 16, 2020

Hi and thanks for the great package.

I have hard time understanding what goes wrong. For some reason I can't get the default data for bootstrapping etc to be logged and present in Xray. I looked into the source code and found that it is because

    /**
     * @param SegmentSubmitter $submitter
     */
    public function submit(SegmentSubmitter $submitter)
    {
        if (!$this->isSampled()) {
            return;
        }

        $submitter->submitSegment($this);
    }

$this->isSampled() is always false. I saw in your package you set it from the header

    ->setTraceHeader($_SERVER['HTTP_X_AMZN_TRACE_ID'] ?? null)

You look for the part Sampled=1 but when I log this header in my lama function the header always contains only the Root field and nothing else. I've tried to add a sample rule that matches everything with higher rate, but with no luck. Also the AWS documentation on this header says the only valid parts are Root and Self.

Can you please tell me from where we expect this Sampled part to be present in the HTTP_X_AMZN_TRACE_ID header and how to make the default logging for every request to work?

Thanks

@StevePorter92
Copy link
Contributor

Are you using an application load balancer for the lambda trigger? It looks like trace id's from ALBs only support the Root and Self key so the underlying library won't correctly set the sampling from the trace header. https://docs.aws.amazon.com/elasticloadbalancing/latest/application/load-balancer-request-tracing.html

I've got around this by forking the library and sampling all requests

trybeapp@daec8a0

# composer.json
...
    "require": {
        "napp/xray-laravel": "dev-master#daec8a0087508583d854a50449fad1702495e3f4",
....
    "repositories": [
        {
            "type": "git",
            "url": "https://github.com/trybeapp/xray-laravel"
        },

@viezel
Copy link
Contributor

viezel commented Aug 5, 2020

@StevePorter92 if you sample all requests then you cannot turn X-ray off.

@viezel
Copy link
Contributor

viezel commented Aug 5, 2020

@Sh1d0w you need to enable X-ray for sampled=1 to work.
You can do the hack suggested, but be aware that you will need to remove the package again to stop tracing since it’s always true.

@viezel viezel closed this as completed Aug 5, 2020
@StevePorter92
Copy link
Contributor

StevePorter92 commented Aug 5, 2020

Is it worth raising a PR to wrap the segment submission around the enabled flag in the RequestTracing middleware?

    /**
     * Terminates a request/response cycle.
     */
    public function terminate($request, $response)
    {
        if ($this->xray->isEnabled()) {
            $this->xray->submitHttpTracer($response);
        }
    }

I had assumed that that is what was happening but looks like you are right. The xray.enabled config just prevents some of the collectors from being registered

@brajnacs
Copy link

I am trying to get it running on an ECS container with APISegmentSubmitter with no luck. Looking at the code in SegmentCollector I can see that the sampling is relying on the HTTP_X_AMZN_TRACE_ID while the actual header is X-Amzn-Trace-Id.

X-Amzn-Trace-Id is the header name in the official documentation. Is there a reason/explanation of why the package is expecting HTTP_X_AMZN_TRACE_ID ?

@digitalcraft
Copy link

I can see this got closed, but nothing got merged.
This is definitely still an issue, is there some different config for running on lambda other than Steves "hack"?

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

5 participants