[Serverless] Enable OTLP endpoint in extension.#15321
Conversation
| DisableGracefulShutdown: true, | ||
| ConfigProvider: configProvider, | ||
| LoggingOptions: options, | ||
| SkipSettingGRPCLogger: true, |
There was a problem hiding this comment.
See comment on commit 3f4a78e for more details on this change.
There was a problem hiding this comment.
Please add the text from the description of the commit you've linked above as an inline comment. Alternatively, a link to the commit message itself will also do.
If any issue exists upstream about this problem (?) of locking it would be nice to link to that too. If not, does it make sense to open one?
| // Concurrently start heavyweight features | ||
| var wg sync.WaitGroup | ||
| wg.Add(3) | ||
| wg.Add(4) |
There was a problem hiding this comment.
I would recommend pairing a wg.Add(1) directly above every go func call to pair it with it's wg.Done() counterpart to avoid any counting mistakes later on.
| DisableGracefulShutdown: true, | ||
| ConfigProvider: configProvider, | ||
| LoggingOptions: options, | ||
| SkipSettingGRPCLogger: true, |
There was a problem hiding this comment.
Please add the text from the description of the commit you've linked above as an inline comment. Alternatively, a link to the commit message itself will also do.
If any issue exists upstream about this problem (?) of locking it would be nice to link to that too. If not, does it make sense to open one?
411874a to
27bb5f8
Compare
rtrieu
left a comment
rtrieu
left a comment
There was a problem hiding this comment.
Minor feedback, otherwise looks good.
3268de4 to
aef8ffd
Compare
6cb726a to
7d46d12
Compare
|
FYI. Related PR to build serverless extensions with the |
7d46d12 to
dbe51b3
Compare
gbbr
left a comment
gbbr
left a comment
There was a problem hiding this comment.
LGTM as far as pkg/otlp goes.
There was a problem hiding this comment.
I don't think this warrants its own package. It's a very small one-off thing used exclusively in one place. Furthermore, it creates clashing with another existing package (in name). However, I don't feel strongly about this and will leave it completely up to you to decide what to do here.
mx-psi
left a comment
mx-psi
left a comment
There was a problem hiding this comment.
LGTM, added team/opentelemetry since we should also test our pipeline during QA to avoid regressions
…54599.yaml Co-authored-by: Rosa Trieu <[email protected]>
0382990 to
41ad50b
Compare
cbccb06 to
7cc4077
Compare
…adog-agent into rey.abolofia/serverless-otlp
6 participants
What does this PR do?
This adds support for enabling the OTLP endpoint when using the aws lambda extension. The configuration for turning this feature on is the same as it is for the regular agent.
Motivation
Additional Notes
Possible Drawbacks / Trade-offs
Describe how to test/QA your changes
Reviewer's Checklist
Triagemilestone is set.major_changelabel if your change either has a major impact on the code base, is impacting multiple teams or is changing important well-established internals of the Agent. This label will be use during QA to make sure each team pay extra attention to the changed behavior. For any customer facing change use a releasenote.changelog/no-changeloglabel has been applied.qa/skip-qalabel is not applied.team/..label has been applied, indicating the team(s) that should QA this change.need-change/operatorandneed-change/helmlabels have been applied.k8s/<min-version>label, indicating the lowest Kubernetes version compatible with this feature.