-
Notifications
You must be signed in to change notification settings - Fork 28
CEXT-5047: Create a documentation how to use observability module #434
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
CEXT-5047: Create a documentation how to use observability module #434
Conversation
|
@oshmyheliuk i've made a few changes, if you could review the last few commits and make sure everything still looks good to you. |
| "destination": "NewRelic", | ||
| "destination_endpoint": "https:\/\/log-api.newrelic.com\/log\/v1 ", | ||
| "destination_api_key": "********", | ||
| "destination_api_key": "abcd1234-abcdabc-afs", |
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.
Here it would actually be "********" as we are masking secrets in the REST response with asterics.
|
@jhadobe changes look good, thank you. I left one comment. As @keharper mentioned, this documentation is planned to be combined with the APP Builder observability documentation (probably this PR https://github.com/AdobeDocs/commerce-extensibility/pull/413/files). So, I don't know if the structure of the pages in this PR is compatible with that PR. |
Thanks @oshmyheliuk I've addressed your comment. Regarding #413 we are not sure right now where that PR will actually go. It may end up as part of a completely different repo in the App Builder documentation, because it is not specific to Commerce (unlike this PR). Wherever it ends up, I'll make sure it is integrated with the content from this PR. |
|
Converting this to a draft until the functionality is moved to production. |
| const instrumentedMain = instrumentEntrypoint(main, { | ||
| ...telemetryConfig, | ||
| propagation: { | ||
| getContextCarrier: (params) => { | ||
| if (params && params.__ow_headers && params.__ow_headers["traceparent"]) { | ||
| return { | ||
| carrier: { | ||
| traceparent: params && params.__ow_headers && params.__ow_headers["traceparent"] || "", | ||
| tracestate: params && params.__ow_headers && params.__ow_headers["tracestate"] || "" | ||
| } | ||
| } | ||
| } else { | ||
| return { | ||
| carrier: {} | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }); | ||
| exports.main = instrumentedMain |
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.
Before making this recommendation I'd actually prefer to build this directly in aio-lib-telemetry, following the specification we spoke about. Please let me know if this is prioritary and I'll let my managers know so they can help prioritize that change.
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.
We can add this example for now. And to remove this example after the implementation of that logic in aio-lib-telemetry.
If the implementation would be compatible with this current approach.
| const instrumentedMain = instrumentEntrypoint(main, { | ||
| ...telemetryConfig, | ||
| propagation: { | ||
| getContextCarrier: (params) => { | ||
| if (params.data._metadata && params.data._metadata["traceparent"]) { | ||
| return { | ||
| carrier: { | ||
| traceparent: params.data._metadata["traceparent"] || "", | ||
| tracestate: params.data._metadata["tracestate"] || "" | ||
| } | ||
| } | ||
| } else { | ||
| return { | ||
| carrier: {} | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }); | ||
| exports.main = instrumentedMain |
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.
This other example I'm okay with, as I don't feel comfortable baking Commerce-specific features (params.data._metadata) into aio-lib-telemetry.
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.
I'm ok with not adding it to aio-lib-telemetry.
However, on the other hand, aio-lib-telemetry was created for Commerce, and I don't see it as a significant issue to add a check for event metadata. But it's totally up to you.
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.
We created it in the Commerce org as an experiment that's true, but it's usage it's not limited to it, that's why I'd prefer not. But I think we can abstract it away in multiple other ways without actually coupling the two.
| You can run observability locally to test Commerce extensibility tools and their connection to App Builder. Refer to the [how to run Grafana locally](https://github.com/adobe/aio-lib-telemetry/blob/main/docs/use-cases/grafana.md#local-development) example for more information. | ||
|
|
||
| You can use a tunneling service to forward logs to your local development machine from the Commerce instance and deployed App Builder action. For example, you can use [Ngrok](https://ngrok.com/) to expose your local development environment to the internet. For more information, refer to [tunnel forwarding](https://github.com/adobe/aio-lib-telemetry/blob/main/docs/use-cases/support/tunnel-forwarding.md). | ||
|
|
||
|  | ||
|
|
||
| To filter logs for a single request you can use the `trace_id` filter. The `trace_id` is propagated from Commerce to App Builder. Use the `trace_id` to correlate logs from both systems. | ||
|
|
||
|  |
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.
Perhaps it's best to directly reference or forward to the full explanation in the aio-lib-telemetry, this way we don't have fragmented and duplicated docs.
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.
Yes, I thought to update it with a link to a public documentation for aio-lib-telemetry in devdocs when it would be published.
Updated the observability documentation for clarity and consistency, including corrections to terminology and formatting.
Purpose of this pull request
This pull request (PR) adds information on how to use Commerce Observability in connection with App Builder.
https://jira.corp.adobe.com/browse/CEXT-5047
Affected pages
Links to Magento Open Source code