- 
                Notifications
    You must be signed in to change notification settings 
- Fork 35
Breadcrumbs support #83
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
Conversation
…ent from sending nullable records
…bs/backtrace-unity into feature/breadcrumbs
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 think there is a logical bug in: BacktraceInMemoryLogManager.cs
        
          
                Runtime/Model/Breadcrumbs/Storage/BacktraceStorageLogManager.cs
              
                Outdated
          
            Show resolved
            Hide resolved
        
      | var jsonObject = CreateBreadcrumbJson(id, message, level, type, attributes); | ||
| bytes = System.Text.Encoding.UTF8.GetBytes(jsonObject.ToJson()); | ||
|  | ||
| if (currentSize + bytes.Length > BreadcrumbsSize) | 
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 think we only need to lock this part, we can save a lot of time by avoiding unnecessary locking, especially on every Add 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.
I had the same feeling but on the other hand, what should we do when we're in the middle of cleaning breadcrumbs? We will remove the new breadcrumb, right?
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.
Looks good, no major issues spotted!
* Divided commit into two branches for reviewers. Added attribute provider and Backtrace http client for testing purpose. * Adjusted labels * Start default event on the application startup + send empty string instead of null * Event aggregation logic for reviewers (#74) * Event aggregation logic for reviewers * Adjusted the most flexible cases * Code review suggestions * Default event support * backoff for session events when request failed * Retry only on 503 * Exponential backoff * Avoid casting to float, fixed Math.Pow * Added Rameez suggestions * Code review suggestions * Fixed wrong retry function argument * line ending fix * Fixed invalid property name after merge * Adjusted nullable JSON values * Create attribute provider dynamically * Enable attribute provider * Disable maximum number of events in store from config * Adjust session aggregation support to spec requirements and meeting suggestions * Fixed typo * git attributes for line endings * Adjust current flow to new requirements + editor changes * Allow for overriding default unique event * Added getter and setter for backtracesesion support * Adjust backtrace session object * Backtrace session API improvements * Safe native attribute ios * Safe native attributes * Adjusted native build * Use empty list of classifiers instead of empty string * Adjusted API * Adjusted API + Session=>Summed + BacktraceSession=>BacktraceMetrics * Adjusted API + Session=>Summed + BacktraceSession=>BacktraceMetrics * Renamed ms to sec * Event aggregation support (#79) * Divided commit into two branches for reviewers. Added attribute provider and Backtrace http client for testing purpose. * Adjusted labels * Start default event on the application startup + send empty string instead of null * Event aggregation logic for reviewers (#74) * Event aggregation logic for reviewers * Adjusted the most flexible cases * Code review suggestions * Default event support * backoff for session events when request failed * Retry only on 503 * Exponential backoff * Avoid casting to float, fixed Math.Pow * Added Rameez suggestions * Code review suggestions * Fixed wrong retry function argument * line ending fix * Fixed invalid property name after merge * Adjusted nullable JSON values * Create attribute provider dynamically * Enable attribute provider * Disable maximum number of events in store from config * Adjust session aggregation support to spec requirements and meeting suggestions * Fixed typo * git attributes for line endings * Adjust current flow to new requirements + editor changes * Allow for overriding default unique event * Added getter and setter for backtracesesion support * Adjust backtrace session object * Backtrace session API improvements * Safe native attribute ios * Safe native attributes * Adjusted native build * Use empty list of classifiers instead of empty string * Adjusted API * Adjusted API + Session=>Summed + BacktraceSession=>BacktraceMetrics * Adjusted API + Session=>Summed + BacktraceSession=>BacktraceMetrics * Renamed ms to sec * Fixed stucture size (#80) * Guess native library path when application context is not available (#81) * Guessing lib path based on the current game directory when the activity is not available * Code reformat * Renamed ms to sec * Handle different unity player java class name (#87) * Removed warnings generated by Unity2020 + Unity2021 (#85) * Fixed problem with Unity2021 - symbols upload warnings on the android upload symbols task * Removed warnings generated by Unity2020 * Fixed compatibility issues * Formatting * Metrics support imrovements (#88) * divided summed/unique events into different queues * Fully worked unit tests * adjusted url + json keys * Code review feedback * Breadcrumbs support (#83) * Breadcrumbs support: before database operations cleanup * Breadcrumbs improvements * Database adjustements * Adjusted database code * Removed try/catch block from function that handles exception and prevent from sending nullable records * Allow nullable record in SendData method * Squashed commit of the following: commit 8c2cec9 Author: kdysput <konrad.dysput@gmail.com> Date: Thu May 6 00:00:10 2021 +0200 Adjusted nullable JSON values commit 2ed2d7f Author: kdysput <konrad.dysput@gmail.com> Date: Wed May 5 23:55:47 2021 +0200 Fixed invalid property name after merge * Undo session aggregation changes * Create attribute provider dynamically * Undo maximum number of events * Undo attribute provider management * Fixed labels * Expose enableBreadcrumbs API * Fixed name * breadcrumbs tests * Breadcrumbs unit tests - breadcrumb file tests * Renamed enums * Log cleanup tests * Size consistency test * Add stack trace attribute only to error/exception logs * Adjusted native build * Safe native attributes * Safe native attribute ios * Fixed timestamp comparision * Fixed missing namespace + line endings * API update + removed two low memory warnings * breadcrumbs adjustements * Unit tests that validates navigation and logs events * Renamed AddBreadcrumb to Log * Send unique attachments (or include (#n) to attachment name) * Fixed condition + handling empty nullable files * Order beadcrumbs * breadcrumbs support - fixed ios side (delay breadcrumbs startup) * Removed database checks * added breadcrumbs.lastId + offline db support * removed early access flag * Fixed breadcrumb structure + unit tests * typos + new lines updates * Adjusted formating * Adjusted if statement * Removed extra line * COde cleanup - adjustemenets after CR * Fixed compilation issue * Don't dispose before checking network status * Use UTC timestamp * Removed universe ane token submission URL - allow user to override them (#89) * fixed line endings * Editor configuraiton improvements - foldout improvements * Apply default log file size * Label change * Final ios library * Use copy of attributes * Fixed reference shared between native client and managed client * applicaiton.session attribute * Limit number of events to 50 * Use breadcrumbs as double not long * Use only one variable to determinate maximum number of elements in queue * Setting maximum number of events per unique/summed events + breadcrumbId type change * Getter/Setter api interface improvements * Breadcrumbs improvements -> changed types + metrics expose maximum number of events + adjust api to new requiremenets * Unit tests for 502 status code + disabled auto send, long => uint api change, adjusted metrics initialization flow * Changed breadcurmb type: http => system for internet connection check * Read application version and name from cache to add support for background threads * Block WebGL * Calculate retry based on the time * version update * Update CHANGELOG.md * Update CHANGELOG.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update CHANGELOG.md Minor edits * TOC Update for Breadcrumbs and Crash Free Metrics * Ignore LogType.Error (#92) * Ignore errors * Adjusted comment * Screenshot management (#91) * Screenshot management * Changed default screenshot quality * Ratio typo * version bump: preview.2 update * Adjusted attachments + breadcrumbs support * Allow overriding default attributes * Add missing namespace * Application.session + api adjustements backtrace metrics * empty json object adjustement * Windows native dump event aggregation attributes * Correct unit test namespace * Convert device attributes to scoped attributes + include native attributes before native integration start * Breadcrumbs log level tooltip adjustement * Native upload handler tests * Version 3.5.0: Version update * Update README.md * Update README.md * Update CHANGELOG.md * Update README.md * Label change Co-authored-by: Vincent Lussenburg <vlussenburg@users.noreply.github.com> Co-authored-by: jasoncdavis0 <jdavis@backtrace.io>
Why
This diff enables breadcrumbs in the backtrace-unity package. Via breadcrumbs, users can collect unity engine/environment/game/dependency specific logs that will be included in backtraceReports available in Backtrace.
Breadcrumbs are collected via Unity-specific interfaces and also expose an interface for adding manually breadcrumbs via BacktraceAPI. All breadcrumbs will be available in the file that will be uploaded to server.
This diff still shows work in progress. This diff still requires to cleanup database dependency to cleanup/prevent removing breadcrumbs from the database flow. This feature will require to move database record behavior to
BacktraceDatabaseFileContextand forceBacktraceDatabaseContextto cooperate withBacktraceDatabaseFileContext