-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[load-generator] Update locustfile for logging with TraceContext #2265
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Shenoy Pratik <pshenoy36@gmail.com>
Signed-off-by: Shenoy Pratik <pshenoy36@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @ps48 thanks for taking care of that.
I've left some comments, basically to remove the resource
and rely on the service name coming from the env var, and asking to keep just one log entry per call.
Just to not explode the amount of data produced by the load-gen.
src/load-generator/locustfile.py
Outdated
resource = Resource.create({ | ||
"service.name": "load-generator", | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's rely on the env var for that:
https://github.com/open-telemetry/opentelemetry-demo/blob/main/docker-compose.yml#L421C7-L421C41
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense removed the resource attribute injection.
logging.info(f"User {user} performing single item checkout") | ||
self.add_to_cart(user=user) | ||
checkout_person = random.choice(people) | ||
checkout_person["userId"] = user | ||
self.client.post("/api/checkout", json=checkout_person) | ||
logging.info(f"Checkout completed for user {user}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the new spans already increased a lot the amount of data leaving the load-gen service, let's keep just one log entry per call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, just kept the final log statement for each entry. db6f18a
Signed-off-by: Shenoy Pratik <pshenoy36@gmail.com>
Hi @julianocosta89, this commit db6f18a should address all the mentioned changes. |
Changes
Related issue: #2250
Merge Requirements
For new features contributions, please make sure you have completed the following
essential items:
CHANGELOG.md
updated to document new feature additionsMaintainers will not merge until the above have been completed. If you're unsure
which docs need to be changed ping the
@open-telemetry/demo-approvers.