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

Add OTEL & improve logs in Segment app #1675

Merged
merged 2 commits into from
Jan 8, 2025
Merged

Conversation

krzysztofzuraw
Copy link
Member

@krzysztofzuraw krzysztofzuraw commented Jan 8, 2025

Scope of the PR

  • Added OTEL
  • Improved logs in Segment app

Related issues

Checklist

@krzysztofzuraw krzysztofzuraw requested a review from a team as a code owner January 8, 2025 14:19
@krzysztofzuraw krzysztofzuraw requested review from a team and lkostrowski January 8, 2025 14:19
Copy link

changeset-bot bot commented Jan 8, 2025

🦋 Changeset detected

Latest commit: cd4c973

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
segment Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

vercel bot commented Jan 8, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
saleor-app-avatax ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 8, 2025 2:29pm
saleor-app-cms ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 8, 2025 2:29pm
saleor-app-klaviyo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 8, 2025 2:29pm
saleor-app-products-feed ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 8, 2025 2:29pm
saleor-app-search ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 8, 2025 2:29pm
saleor-app-segment ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 8, 2025 2:29pm
saleor-app-smtp ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 8, 2025 2:29pm

Comment on lines +54 to +56
logger.warn(
"Segment write key is not set in app configuration. Contact client to fix the issue.",
);
Copy link
Member

Choose a reason for hiding this comment

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

question: do we want to treat broken app config as warning for our logs? I think this is fine for now, but imho it's not really scalable and client logs would be better ;)

Copy link
Member Author

@krzysztofzuraw krzysztofzuraw Jan 8, 2025

Choose a reason for hiding this comment

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

In other PR I disable webhooks is there is no key so hopefully there won't be many such situations but I agree - we should let client know (client logs or maybe emails etc.)

@krzysztofzuraw krzysztofzuraw merged commit ab042d4 into canary Jan 8, 2025
19 checks passed
@krzysztofzuraw krzysztofzuraw deleted the shopx-1748-otel-logs-1 branch January 8, 2025 14:46
lkostrowski added a commit that referenced this pull request Jan 13, 2025
* log suspicious taxes calculation (#1658)

* fix log (#1660)

* Use vercel log drain in merchant apps (#1657)

* Use vercel log drain

* Add changeset

* improve log for non zero line (#1663)

* Bring back Segment app to monorepo (#1665)

* Fix deployment of Segment app (#1666)

* Add setup node to GHA workflow (#1673)

* Add Sentry to Segment app (#1671)

* Add OTEL & improve logs in Segment app (#1675)

* Fixed app version send to Segment & how we send events (#1676)

* Add use-case to Segment app (#1677)

* Initially disable Segment app webhooks (#1678)

* Add README to Segment app (#1683)

* Improve Segment app logo (#1682)

* Fix missing cache for test workflow (#1685)

* Add new Vercel log limit (#1684)

* Run autofix for monorepo (#1681)

* Fix code scan issue with ALLOWED_DOMAIN_PATTERN regex (#1687)

---------

Co-authored-by: Lukasz Ostrowski <[email protected]>
Co-authored-by: Paweł Chyła <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants