-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Change AccountService from Go to DotNet (auto) #1538
Change AccountService from Go to DotNet (auto) #1538
Conversation
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
@RassK Can you add a Chanlog entry for this? Also, we will need to update docs and the Helm chart for this. If you need help to prep a PR in those areas, please let us know. |
@puckpuck thanks for helping to guide the last steps missing. I have updated the changelog. Would be glad to accept some help with other missing parts as well. It's my first PR to this repo. |
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
# Conflicts: # CHANGELOG.md # src/accountingservice/go.mod # src/accountingservice/go.sum
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 @RassK it looks good, before approving would you mind explaining those log entries?
Accounting service started
[CORECLR_ENABLE_PROFILING, 1]
[CORECLR_PROFILER, {918728DD-259F-4A6A-AC2B-B85E1B658318}]
[CORECLR_PROFILER_PATH, /app/OpenTelemetry.AutoInstrumentation.Native.so]
[DOTNET_RUNNING_IN_CONTAINER, true]
[DOTNET_STARTUP_HOOKS, /app/OpenTelemetry.AutoInstrumentation.StartupHook.dll]
That's a quick startup diagnostics (by Ideally a built in diagnostics (OTEL_LOG_LEVEL=debug) should do the similar but instead logs to the file. |
I've updated the tracetest to reflect the change, hope you don't mind. 😅 This should be good to go, I'll just wait to see if any other maintainer/approver has something to say. Thanks for taking care of it 🤩 |
Just need to increase mem limit in the helm chart left? Can anyone help / explain where, I have had 0 connections to helm charts so far 😬 |
the Helm chart will not be updated till next version release. If you want to update it, you can do the memory limit here: Then you need to bump a minor version here: https://github.com/open-telemetry/opentelemetry-helm-charts/blob/main/charts/opentelemetry-demo/Chart.yaml#L4 And last you need to run the: You could also sync with @rogercoll and add it to his opened PR: Whenever you add it, you will still need to run the |
I tested again, and all looks fine using a linked span here instead. I will make another PR so the frauddetectionservice goes back to creating a child span instead of a linked span. This will allow us to showcase both examples with a message queue. |
* Change AccountService from go to dotnet (auto) * fix path * fix folder name * dockerfile and other fixes * add copyright * fix encoding and cleanup * Cleanup dockerfile * Update OTel Auto * fix kafka processing issues and otel export * remove eof * Update changelog * Use default CancellationDelayMaxMs * update packages * fix merge failure * Fix tracetest 'accountingservice' is not part of the trace anymore --------- Co-authored-by: Juliano Costa <juliano.costa@datadoghq.com> Co-authored-by: Juliano Costa <julianocosta89@outlook.com>
* Change AccountService from go to dotnet (auto) * fix path * fix folder name * dockerfile and other fixes * add copyright * fix encoding and cleanup * Cleanup dockerfile * Update OTel Auto * fix kafka processing issues and otel export * remove eof * Update changelog * Use default CancellationDelayMaxMs * update packages * fix merge failure * Fix tracetest 'accountingservice' is not part of the trace anymore --------- Co-authored-by: Juliano Costa <juliano.costa@datadoghq.com> Co-authored-by: Juliano Costa <julianocosta89@outlook.com>
* Change AccountService from go to dotnet (auto) * fix path * fix folder name * dockerfile and other fixes * add copyright * fix encoding and cleanup * Cleanup dockerfile * Update OTel Auto * fix kafka processing issues and otel export * remove eof * Update changelog * Use default CancellationDelayMaxMs * update packages * fix merge failure * Fix tracetest 'accountingservice' is not part of the trace anymore --------- Co-authored-by: Juliano Costa <juliano.costa@datadoghq.com> Co-authored-by: Juliano Costa <julianocosta89@outlook.com>
Changes
Fixes #589
Changes AccountingService from Go to .NET service.
Uses OpenTelemetry .NET Automatic Instrumentation.
NOTE: It's a first preview of demo app that is instrumented with OpenTelemetry .NET Automatic instrumentation. Some parts are still TBD. Please review and add comments about improvements and missing parts / docs.
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.