Drukowana wersja tematu

Kliknij tu, aby zobaczyć temat w orginalnym formacie

Forum PHP.pl _ Oceny _ Wstepna wersja loggera do oceny

Napisany przez: q.michal 18.03.2016, 00:04:27

Hej,

W koncu znalazlem troche czasu aby dokonczyc wstepna wersje wlasnej implementacji loggera, ktora pragne sie z Wami podzielic. Jak zwykle licze na konstruktywna krytyke, a jednoczesnie chcialbym takze zapytac bardziej doswiadczonych PHP-owcow o jeden detal. Mianowicie wywolanie date('u'); zwraca same zera zamiast mikrosekund. Udalo sie moze komus to naprawic?


Kod do wgladu: http://wklej.org/hash/b0997746849/

Napisany przez: KsaR 18.03.2016, 00:23:35

Skasuj "? true : false" bo to bez sensu,
(prawda lub falsz) ? Prawda : Falsz;

Co do problemu -
http://stackoverflow.com/questions/169428/php-datetime-microseconds-always-returns-0

Napisany przez: q.michal 18.03.2016, 00:40:19

Racja, poprawilem.
Co do problemu z data, to mam rozumiec, ze to poprostu nie dziala w PHP i przez tyle wersji nie zostalo poprawione, nawet w najnowszym PHP7 z ktorego korzystam?
Czyli jedynym sposobem jest napisanie wrapper dla funkcji date()?

Napisany przez: KsaR 18.03.2016, 00:43:43

Cytat(q.michal @ 18.03.2016, 00:40:19 ) *
Racja, poprawilem.
Co do problemu z data, to mam rozumiec, ze to poprostu nie dziala w PHP i przez tyle wersji nie zostalo poprawione, nawet w najnowszym PHP7 z ktorego korzystam?
Czyli jedynym sposobem jest napisanie wrapper dla funkcji date()?

Możliwe że zapomnieli naprawić tongue.gif
Bo faktycznie odapliłem teraz na php 7.0.4
[PHP] pobierz, plaintext 
  1. http://www.php.net/echo http://www.php.net/date('u');
[PHP] pobierz, plaintext

I zwraca 000000.
--
A jako że wspierasz (chyba) php 5.3 i (nizsze?) np. array() zamiast []...
To wrapper może być skoro nadal nie "naprawili" może to kwestia konfiguracji serwera ale nie mam pewności.

Napisany przez: Pyton_000 18.03.2016, 14:12:12

getHandlersList() i getProcessorsList() - 98% kodu copy&paste

Napisany przez: q.michal 18.03.2016, 16:36:45

Cytat(Pyton_000 @ 18.03.2016, 14:12:12 ) *
getHandlersList() i getProcessorsList() - 98% kodu copy&paste


Tak, obecnie tak.
Na razie tak. Jak mowilem, jest to wstepna wersja.
Poza tym niemal identyczna metode znalezc mozna w klasie implementujacej obsluge cache.
Musze przepisac ten kod aby byl bardziej generyczny i wydzielic do osobnej klaski, z ktorej te beda korzystac.
Wiem o tym, niemniej dzieki za uwage wink.gif

Napisany przez: com 18.03.2016, 17:39:08

skoro jesteś tego świadomy to czemu od razu piszesz źle?

Poza tym w php już od pewnego czasu mamy standardy i tworzenie kolejnej biblioteki, nie zgodnej z nim mija się z najmniejszym celem.
http://www.php-fig.org/psr/psr-3/

Co do pytania, czytało by się dokumentacje to by się wiedziało:

Cytat
Microseconds (added in PHP 5.2.2). Note that date() will always generate 000000 since it takes an integer parameter, whereas DateTime::format() does support microseconds if DateTime was created with microseconds.

http://us.php.net/manual/en/function.date.php


http://ideone.com/IliK1Y

Napisany przez: q.michal 18.03.2016, 18:31:27

Jak juz napisalem, jest to *WSTEPNA* wersja.
Co do PSR - owszem, mozna wszystko do tego sprowadzac, co nie znaczy ze jest idealny pod kazdym wzgledem.
A przykladu daleko szukac nie trzeba:

[PHP] pobierz, plaintext 
  1. <?php
  2.     /**
  3.      * System is unusable.
  4.      *
  5.      * @param string $message
  6.      * @param array $context
  7.      * @return null
  8.      */
  9.     public function emergency($message, http://www.php.net/array $context = http://www.php.net/array());
  10.  
  11.     /**
  12.      * Action must be taken immediately.
  13.      *
  14.      * Example: Entire website down, database unavailable, etc. This should
  15.      * trigger the SMS alerts and wake you up.
  16.      *
  17.      * @param string $message
  18.      * @param array $context
  19.      * @return null
  20.      */
  21.     public function alert($message, http://www.php.net/array $context = http://www.php.net/array());
[PHP] pobierz, plaintext


Skoro alert wymaga obudzenia kogos kto naprawi problem i opisany jest jako database unavailable czy entire website down, to moim zdaniem gorzej byc nie moze. Chyba, ze caly serwer padnie, ale wtedy ten emergency na nic Ci sie nie przyda. Poza tym jezeli zatrzyma sie baza danych, to notyfikacja powinna przyjsc z innego zrodla. Inna sprawa, ze jak cala strona bedzie nieosiagalna, to z duzym prawdopodobienstwem tej notyfikacji nie dostaniesz bo skrypt moze sie w ogole nie wykonac.
Od tego masz Icinge, Zabbixa i inne profesjonalne narzedzia. Nie ma wiec sensu bezmyslnie podazac za wszystkimi trendami i standardami.

Odnosnie daty - w konstruktorze przekazujesz date z mikrosekundami, a ja chce poznac aktualny czas.

Napisany przez: destroyerr 18.03.2016, 19:33:05

Cytat
to moim zdaniem gorzej byc nie moze

Ktoś może mieć inne zdanie, dlatego są różne poziomy i w danym systemie dla danego zdarzenia przyjmujesz określony poziom.
Cytat
Chyba, ze caly serwer padnie, ale wtedy ten emergency na nic Ci sie nie przyda.

Co jeśli mam wiele serwerów?
Cytat
Poza tym jezeli zatrzyma sie baza danych, to notyfikacja powinna przyjsc z innego zrodla.

Nie rozumiem, np. z jakiego źródła jak nie ze skryptu?

Cytat
Nie ma wiec sensu bezmyslnie podazac za wszystkimi trendami i standardami.

Za trendami to masz rację, że nie warto bezmyślnie podążać. Natomiast trend a standard to kompletnie różne rzeczy. Pewnie przestrzenie nazw to też jakiś dziwny standard, ale niekorzystanie z niego prowadzi do takich kwiatków:
[PHP] pobierz, plaintext 
  1. $class = 'LoggerProcessor' . http://www.php.net/ucfirst($processor);
[PHP] pobierz, plaintext

Jeszcze w kwestii bezmyślnego podążania, to kopiowanie bibliotek też się w to wlicza? Twój kawałek kodu moim zdaniem zbyt mocno przypomina inny.

Stałe jak QEVENT_WARNING zdefiniowałbym w zasięgu klasy. Statyczne zależności i magiczne stałe. Abstrakcyjna klasa z pustym konstruktorem i destruktorem. Kod ogólnie trudny do rozbudowy i to chyba największa wada.

Napisany przez: q.michal 18.03.2016, 20:24:17

Cytat(destroyerr @ 18.03.2016, 19:33:05 ) *
Ktoś może mieć inne zdanie, dlatego są różne poziomy i w danym systemie dla danego zdarzenia przyjmujesz określony poziom.

Co jeśli mam wiele serwerów?

To wtedy powinienes dostac notyfikacje z klastra lub innego systemu zapewniajacego HA.
Rozroznij bledy w skrypcie, od bledow w aplikacji / serwerze.

Cytat(destroyerr @ 18.03.2016, 19:33:05 ) *
Nie rozumiem, np. z jakiego źródła jak nie ze skryptu?


Tak jak napisalem: klaster, monitoring, etc.

Cytat(destroyerr @ 18.03.2016, 19:33:05 ) *
Jeszcze w kwestii bezmyślnego podążania, to kopiowanie bibliotek też się w to wlicza? Twój kawałek kodu moim zdaniem zbyt mocno przypomina inny.


Ciekawe. Z jednej strony zarzucasz mi ze nie korzystam z przestrzeni nazw itp, a z 2 ze kopiuje biblioteki.

Napisany przez: hind 18.03.2016, 20:53:55

Nie myślałeś o implementacji PSR-3?

Napisany przez: q.michal 18.03.2016, 21:13:33

Myslalem, ale implementacja 20 poziomow logowanai dla samego faktu ich posiadania do mnie nie przemawia.
Jak juz pisalem wczesniej, brak mozliwosci podlaczenia do bazy, czy brak jakiegos komponentu moze generowac ALERTa.
Emergency jest tutaj na prawde zbedny. To jest system monitorowania skryptu, a nie wszystkiego dookola. On moze moze wywolac alert kiedy nie bedzie mogl sie podlaczyc do bazy, ale i tak powinien zadzialac zewnetrzny monitoring i poinformowac administratora ze zatrzymala sie abza danych, ew jakis klaster/watchdog powinien ja automatycznie zrestartowac.

Napisany przez: com 18.03.2016, 23:23:25

Zanim kolejny raz zaczniesz zgrywać mądrego polecam przeczytać całość, tam wyraźnie zostało napisane dlaczego mamy 8 poziomów logowania.

Cytat
The LoggerInterface exposes eight methods to write logs to the eight RFC 5424 levels (debug, info, notice, warning, error, critical, alert, emergency).


Całość oparta jest na standardzie opisany w RFC. To co mówisz owszem, te mechanizmy powinny tam zadziałać, ale jest standard i interfejs musi być z nim zgodny.

Pomijając już sam fakt iż piszesz na serwerze z php7, a nawet przestrzeni nazw z 5.3 nie stosujesz, to powinieneś stosować się do standardu, po to abym jak zechcę mógł Twoją implementację zamienić na np monolog, jeśli stwierdzę, że akurat jego potrzebuję. System nie powinien zależeć od konkretnej implementacji...

A wymyślanie koła na nowo mija się z celem.

Cytat
Jak juz napisalem, jest to *WSTEPNA* wersja.


Jakie to ma znaczenie? Jeśli jesteś dobrym programistą a nie wyglądasz na pierwszego lepszego gimbusa, powinieneś już to wiedzieć iż są programiści i klepacze kodu. Rozróżnić ich można po jakości kodu. Jasne można pisać kod, który nadaje się tylko do refaktoringu, ale to nie jest produktywne.

Cytat
Odnosnie daty - w konstruktorze przekazujesz date z mikrosekundami, a ja chce poznac aktualny czas.


Przeczytałeś cytat z dokumentacji?

Napisany przez: destroyerr 19.03.2016, 14:07:43

Cytat
To wtedy powinienes dostac notyfikacje z klastra lub innego systemu zapewniajacego HA.

A co jeśli jestem programistą i nie interesuje mnie nic poza moją aplikacją? Utrzymywaniem infrastruktury zajmuje się kto inny. Ja chcę wiedzieć czy moja aplikacja zepsuła się i mam coś do naprawy czy nie. Poza tym uczepiłeś się jednego przykładu dla tego poziomu.

Cytat
Rozroznij bledy w skrypcie, od bledow w aplikacji / serwerze.

Właśnie rozróżniam. Skrypt nie może połączyć się z bazą. Może baza padła, albo admin zmienił jej adres? Efekt jest taki, że nie działa aplikacja i ja chcę o tym wiedzieć. Komunikat z klastra nic mi nie mówi bo się na tym mogę nie znać.

Cytat
Ciekawe. Z jednej strony zarzucasz mi ze nie korzystam z przestrzeni nazw itp, a z 2 ze kopiuje biblioteki.

Niczego Ci nie zarzucam tylko opisuję istniejącą sytuację. Po prostu nie korzystasz z przestrzeni nazw i tyle, a to moim zdaniem błąd i to krytykuję (o co zresztą sam prosiłeś). Jaki jest związek między używaniem PN a kopiowaniem cudzej biblioteki nie podając źródła? Wielu programistów korzysta z PN i nie kopiują innych bibliotek.

Napisany przez: q.michal 19.03.2016, 19:39:04

Cytat(destroyerr @ 18.03.2016, 19:33:05 ) *
Za trendami to masz rację, że nie warto bezmyślnie podążać. Natomiast trend a standard to kompletnie różne rzeczy. Pewnie przestrzenie nazw to też jakiś dziwny standard, ale niekorzystanie z niego prowadzi do takich kwiatków:
[PHP] pobierz, plaintext 
  1. $class = 'LoggerProcessor' . http://www.php.net/ucfirst($processor);
[PHP] pobierz, plaintext

Bardzo chetnie dowiem sie, jak to powinno zostac zaimplementowane wg Ciebie. Jezeli masz jakis przyklad, albo chwile aby zaprezentowac fragment kodu, bede wdzieczny.

Cytat(com @ 18.03.2016, 23:23:25 ) *
Pomijając już sam fakt iż piszesz na serwerze z php7, a nawet przestrzeni nazw z 5.3 nie stosujesz, to powinieneś stosować się do standardu, po to abym jak zechcę mógł Twoją implementację zamienić na np monolog, jeśli stwierdzę, że akurat jego potrzebuję. System nie powinien zależeć od konkretnej implementacji...


Owszem korzystam z PHP7, co nie nakazuje mi w zaden sposob korzystac ze wszystkich dobrodziejstw jakie oferuje najnowsza wersja. PHP jest kompatybilne wstecz w pewnym zakresie, wiec aplikacje ktora wymaga min. PHP5 w teorii da sie uruchomic na PHP7 (ew. po pewnych poprawkach). Z najnowszej wersji interpretera korzystam, bo taka jest oferowana jako stabilna w dystrybucji z ktorej korzystam (tak mam Linuksa). Strata czasu byloby maskowac nowa wersje i robic downgrade, w szczegolnosci ze siodemka jest zdecydowanie szybsza.

Cytat(com @ 18.03.2016, 23:23:25 ) *
A wymyślanie koła na nowo mija się z celem.

"Wszyscy wiedzą, że czegoś nie da się zrobić, aż znajduje się taki jeden, który nie wie, że się nie da, i on to robi."

Cytat(destroyerr @ 19.03.2016, 14:07:43 ) *
A co jeśli jestem programistą i nie interesuje mnie nic poza moją aplikacją? Utrzymywaniem infrastruktury zajmuje się kto inny. Ja chcę wiedzieć czy moja aplikacja zepsuła się i mam coś do naprawy czy nie. Poza tym uczepiłeś się jednego przykładu dla tego poziomu.

Rozumiem, ale jak padnie baza danych, to raczej nie Ty bedziesz to naprawial. Jak slusznie zauwazyles kto inny zajmuje sie utrzymywaniem infrastruktury. Osoba taka powinna otrzymac powiadomienie z monitoringu, ze baza danych jest nieosiagalna. Owszem, taki problem powinien zostac zalogowany w samej aplikacji, w szczegolnosci jezeli baza jest dostepna a przykladowo rozsypala sie jakas tabela. Wtedy administratorzy z duza doza prawdopodobienstwa nie otzymaja zadnej notyfikacji, a o zaistnialym problemie dowiedza sie od Ciebie. Co nie zmienia faktu, ze jezeli cos krytycznego stalo sie po stronie aplikacji to w dalszym ciagu jest to alert. A samemu logowaniu aplikacji nie mozna zawierzyc, bo co w sytuacji gdy taka aplikacja nam coredumpuje, albo kiedy wywroci sie serwer www? Wtedy aplikacja bedzie niedostepna, mamy totalny f***up i z zadnych logow aplikacji sie o tym nie dowiesz.

Cytat(destroyerr @ 19.03.2016, 14:07:43 ) *
Właśnie rozróżniam. Skrypt nie może połączyć się z bazą. Może baza padła, albo admin zmienił jej adres? Efekt jest taki, że nie działa aplikacja i ja chcę o tym wiedzieć. Komunikat z klastra nic mi nie mówi bo się na tym mogę nie znać.

I w tym wypadku dostaniesz alert. A jezeli problem lezy po stronie infrastruktury, to i admin powinien dostac stosowne powiadomienie.

Cytat(destroyerr @ 19.03.2016, 14:07:43 ) *
Niczego Ci nie zarzucam tylko opisuję istniejącą sytuację. Po prostu nie korzystasz z przestrzeni nazw i tyle, a to moim zdaniem błąd i to krytykuję (o co zresztą sam prosiłeś). Jaki jest związek między używaniem PN a kopiowaniem cudzej biblioteki nie podając źródła? Wielu programistów korzysta z PN i nie kopiują innych bibliotek.

Nie chce sie klocic, ani wymadrzac. Nie to jest moim celem. Popelnilem fragment kodu i jestem wdzieczny za krytyke. Rozumiem, ze jako programista masz odmienne zdanie co do samej idei. Ja programistom nie jestem, malo tego za moich czasow czlowiek programowac uczyl sie jeszcze w pascalu, ktory niewiele ma juz wspolnego z dzisiejszymi standardami. Na codzien pracuje w IT, ale zajmuje sie czyms zupelnie innym, dlatego pewnie takze moje podejscie do pewnych problemow jest z gola odmienne. Jezeli chodzi o przestrzenie nazw, to rozumiem zauwazony problem i jak bede mial wolna chwile, postaram sie je wdrozyc. Nie rozumiem natomiast, co wg Ciebie skopiowalem?

Powered by Invision Power Board (http://www.invisionboard.com)
© Invision Power Services (http://www.invisionpower.com)