328066 - implement bzip2 compression for sending metrics log

Status: RESOLVED FIXED

(Toolkit Graveyard :: Data Collection/Metrics, defect, P1)

Description

[Brian Ryner (not reading)]

Comment 1

[Darin Fisher]

-> me

Comment 2

[Darin Fisher]

Attachment: v1 patch

Comment 3

[Darin Fisher]

I decided to go the simple route for now and just statically link in the bzip2 library.  I'll probably revise this code once Necko provides a generic compression API.

Comment 4

[Brian Ryner (not reading)]

Comment on attachment 213403 [details] [diff] [review]
v1 patch

>--- src/nsMetricsService.cpp	21 Feb 2006 19:36:50 -0000	1.3
>+++ src/nsMetricsService.cpp	28 Feb 2006 02:36:33 -0000
>@@ -135,94 +143,156 @@ nsMetricsService::LogEvent(const nsAStri
>+NS_IMETHODIMP
>+nsMetricsService::Flush()
>+{
...
>+  PR_Close(fd);
>+  NS_ENSURE_STATE(succeeded);
>+
>+  // Create a new mRoot
>+  rv = CreateRoot();
>+  NS_ENSURE_SUCCESS(rv, rv);
>+

So if writing the data fails for some reason, this will make us retain the data in memory.  I guess that's probably the best way to handle it, since it could be a temporary failure of some sort.

>@@ -233,20 +303,51 @@ nsMetricsService::Observe(nsISupports *s
>+/*static*/ NS_METHOD
>+nsMetricsService::Create(nsISupports *outer, const nsIID &iid, void **result)
>+{

It's not clear to me what this gets us that NS_GENERIC_FACTORY_CONSTRUCTOR_INIT doesn't.

Looks ok otherwise.

Comment 5

[Darin Fisher]

> So if writing the data fails for some reason, this will make us retain the 
> data in memory.  I guess that's probably the best way to handle it, since it 
> could be a temporary failure of some sort.

Yeah, that's true.  I think it's okay.


> >@@ -233,20 +303,51 @@ nsMetricsService::Observe(nsISupports *s
> >+/*static*/ NS_METHOD
> >+nsMetricsService::Create(nsISupports *outer, const nsIID &iid, void **result)
> >+{
> 
> It's not clear to me what this gets us that NS_GENERIC_FACTORY_CONSTRUCTOR_INIT
> doesn't.

I can't use nsMetricsService::get() with NS_GENERIC_FACTORY_CONSTRUCTOR_INIT because nsMetricsService::get() calls do_GetService.  The result would be infinite recursion.  I wrote it that way once before arriving at the current solution.  I'll add a comment if you want ;-)

Comment 6

[Darin Fisher]

fixed-on-trunk

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

doesn't public/nsIMetricsService.idl need a new IID?

Comment 8

[Darin Fisher]

I didn't bother since we haven't released any software with this code.  It isn't built by default.

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

ah, ok.