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

Logger: Fix timestamp and format #5571

Merged

Conversation

acinader
Copy link
Contributor

@acinader acinader commented May 8, 2019

fixes: #5560

When digging into the failed tests on #5561 I realized that we want to configure each transport, rather than the whole logger.

I also think we have to handle json distinctly, so I started a new branch.

@mrowe009 did the hard work here....

@acinader acinader marked this pull request as ready for review May 9, 2019 16:44
@acinader
Copy link
Contributor Author

acinader commented May 9, 2019

closes #5561

@acinader acinader requested a review from dplewis May 9, 2019 16:46
@dplewis dplewis closed this May 9, 2019
@dplewis dplewis reopened this May 9, 2019
@dplewis
Copy link
Member

dplewis commented May 9, 2019

@acinader Looks good to me. Closed/open to force travis to run

@acinader
Copy link
Contributor Author

acinader commented May 9, 2019

fyi, you can just re-run it on travis.

image

@dplewis
Copy link
Member

dplewis commented May 9, 2019

Oops I didn't see that when you moved from Draft to PR.

@acinader
Copy link
Contributor Author

acinader commented May 9, 2019

well good thing you did! looking at the tests that I thought were fixed...
doh!

@codecov
Copy link

codecov bot commented May 9, 2019

Codecov Report

Merging #5571 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5571      +/-   ##
==========================================
- Coverage   94.06%   94.03%   -0.03%     
==========================================
  Files         129      129              
  Lines        9192     9194       +2     
==========================================
  Hits         8646     8646              
- Misses        546      548       +2
Impacted Files Coverage Δ
src/Adapters/Logger/WinstonLogger.js 100% <100%> (ø) ⬆️
src/Adapters/Storage/Mongo/MongoStorageAdapter.js 91.68% <0%> (-0.25%) ⬇️
src/Controllers/DatabaseController.js 94.9% <0%> (-0.19%) ⬇️
...dapters/Storage/Postgres/PostgresStorageAdapter.js 96.9% <0%> (-0.09%) ⬇️
src/ParseServer.js 96.35% <0%> (ø) ⬆️
src/RestWrite.js 93.54% <0%> (+0.17%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dabdf3b...725c831. Read the comment docs.

@dplewis
Copy link
Member

dplewis commented May 9, 2019

Just tried your fix in the dashboard. It doesn't error anymore

Screen Shot 2019-05-09 at 2 36 16 PM

@dplewis dplewis changed the title Logger timestamp fix take 2 Logger: Fix timestamp and format May 9, 2019
@dplewis dplewis merged commit 87da62b into parse-community:master May 9, 2019
@acinader acinader deleted the logger-timestamp-fix-take-2 branch May 9, 2019 20:00
UnderratedDev pushed a commit to UnderratedDev/parse-server that referenced this pull request Mar 21, 2020
* remove no-op config of logger

* add a test to check on the timestamp

* add a test to verify that we
get non json console loggging by default

* configure transports to include
timestamps in files

* Add failing test to confirm that WinstonLoggerAdapter
is not filtering on level.

* actually fix the test to refelect the facth that this isn't the problem

* Remove bogus date ranges that are now failing
becuase we have timestamps.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Logs missing timestamp
2 participants