Skip to content

Disable X-Ray telemetry safely #18

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

Merged
merged 1 commit into from
Apr 14, 2023
Merged

Conversation

joe4dev
Copy link
Member

@joe4dev joe4dev commented Apr 13, 2023

We want to disable telemetry to avoid verbose logging but do so in a safe way without triggering any internal X-Ray panic (nil pointer exception) because telemetry cannot be nil.

Issue

  • Nil pointer exception because telemetry is not initialized panic: runtime **error:** invalid memory address or nil pointer dereference
  • pkg/processor/batchprocessor.go:78 invokes the telemetry API and assumes it is initialized: telemetry.T.SegmentSent(int64(len(batch)))

👉 We need to initialize telemetry (at least with a dummy implementation) because the X-Ray daemon internally depends on it and we preferabbly don’t want to modify the X-Ray daemon itself.

Testing

I manually tested the new init against the customer example from support with both telemetry settings, LOCALSTACK_ENABLE_XRAY_TELEMETRY=1, and disabled by default. No crashes or nil pointer exception occurs anymore.

Background

Commit where we made the telemetry init optional: f65b6ad#diff-1ecaca51fe284265507b331fce50acf4fbf6eaedf32000481d7464b7214ef666R136

Telemetry can be quite verbose (e.g., 10+ requests for a single invocation) and given we currently don't use it, we currently disable it by default. Example PutTelemetryRecords requests:

2023-04-12T22:24:50.907 INFO --- [ asgi_gw_7] localstack.request.aws : AWS xray.PutTelemetryRecords => 200; PutTelemetryRecordsRequest({'TelemetryRecords': [{'Timestamp': datetime.datetime(2023, 4, 12, 20, 24, 50), 'SegmentsReceivedCount': 0, 'SegmentsSentCount': 0, 'SegmentsSpilloverCount': 0, 'SegmentsRejectedCount': 0, 'BackendConnectionErrors': {'TimeoutCount': 0, 'ConnectionRefusedCount': 0, 'HTTPCode4XXCount': 0, 'HTTPCode5XXCount': 0, 'UnknownHostCount': 0, 'OtherCount': 0}}], 'EC2InstanceId': '', 'Hostname': '', 'ResourceARN': ''}, headers={'Host': '192.168.65.2:4566', 'User-Agent': 'aws-sdk-go/1.44.62 (go1.20.3; linux; amd64) exec-env/Aws_Lambda_nodejs18.x xray-agent/xray-daemon/unknown exec-env/Aws_Lambda_nodejs18.x OS/linux-amd64', 'Content-Length': '352', 'Authorization': 'AWS4-HMAC-SHA256 Credential=ASIAQAAAAAAAFGHIIBR3/20230412/us-east-1/xray/aws4_request, SignedHeaders=content-length;content-type;host;x-amz-date;x-amz-security-token;x-amzn-xray-timestamp, Signature=c84e1d1a77b160b8a44b63093ddf7199f4be1992dfd17f1ea9485e0ecdf89bd6', 'Content-Type': 'application/json', 'X-Amz-Date': '20230412T202450Z', 'X-Amz-Security-Token': 'FQoGZXIvYXdzEBYaD3BZkxN+Cybi3GGUVq21pg+hh3d0jznYP8UekdVdu9UgsOUtNomBlc61irAuo21hd2AOjOjT++l/t5Rj8KwCva70BpTcSGiPgE+qAnR8yZVXqSvwZKxR6ppInLVPrDhXryJEsmtOFgG9+Y7+1/6s4QdGfryK7IgtbrIskWOb+TZRC3G5tEOnul3orQlA2/FJyCbYBAp2+6pu+ei7I/ZRLdlL7leafmjVSxda5ZskDn4OJqvYOsgAPQ0VoHTCnd6MLTGplI2DMz7Xp0vriA5VzEvDlg1jEW507W8l8gaq3y7f0MQWee54Hezg2PJIRevyrTYFKtQe9f7UUYrg2gY=', 'X-Amzn-Xray-Timestamp': '1681331090.881712914', 'Accept-Encoding': 'gzip', 'x-localstack-tgt-api': 'xray', 'x-moto-account-id': '000000000000'}); PutTelemetryRecordsResult({}, headers={'Content-Type': 'text/plain; charset=utf-8', 'Content-Length': '2', 'x-amzn-requestid': 'FLC0SER7CFC1RTQW7PPQPEB8M9N215VMTA1JIBJF1JN3INHGZNO2', 'x-amz-request-id': '8YF70O61JSOM8GEUWIG96ARGK42S8LM99IRXV1X6P5Z1L3Y4W1B9', 'Connection': 'close', 'Access-Control-Allow-Origin': '*', 'Access-Control-Allow-Methods': 'HEAD,GET,PUT,POST,DELETE,OPTIONS,PATCH', 'Access-Control-Allow-Headers': 'authorization,cache-control,content-length,content-md5,content-type,etag,location,x-amz-acl,x-amz-content-sha256,x-amz-date,x-amz-request-id,x-amz-security-token,x-amz-tagging,x-amz-target,x-amz-user-agent,x-amz-version-id,x-amzn-requestid,x-localstack-target,amz-sdk-invocation-id,amz-sdk-request', 'Access-Control-Expose-Headers': 'etag,x-amz-version-id'})

We want to disable telemetry to avoid verbose logging but
do so in a safe way without triggering any internal X-Ray panic
because telemetry cannot be nil.
@joe4dev joe4dev requested a review from dominikschubert April 13, 2023 08:55
@joe4dev joe4dev self-assigned this Apr 13, 2023
Copy link
Member

@dominikschubert dominikschubert left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

@joe4dev joe4dev merged commit fc1ba06 into localstack Apr 14, 2023
@joe4dev joe4dev deleted the fix/disable-telemetry-safely branch April 14, 2023 09:33
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.

2 participants