Skip to content

Commit 7040d0a

Browse files
pongadpongad
authored
---
yaml --- r: 7927 b: refs/heads/tswast-patch-1 c: b5f2281 h: refs/heads/master i: 7925: 5b977b5 7923: affb68a 7919: c9e5ae6
1 parent e904f05 commit 7040d0a

3 files changed

Lines changed: 29 additions & 69 deletions

File tree

‎[refs]‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,5 +57,5 @@ refs/tags/v0.18.0: 9d193c4c4b9d1c6f21515dd8e50836b9194ec9bb
5757
refs/tags/v0.19.0: e67b56e4d8dad5f9a7b38c9b2107c23c828f2ed5
5858
refs/tags/v0.20.0: 839f7fb7156535146aa1cb2c5aadd8d375d854e8
5959
refs/tags/v0.20.1: 370471f437f1f4f68a11e068df5cd6bf39edb1fa
60-
refs/heads/tswast-patch-1: afdb204c3529699c8ea3ccc7e32d3795c06e7819
60+
refs/heads/tswast-patch-1: b5f2281e5ada3079a6c6e8d8ebad84cfc3375063
6161
refs/heads/pubsub-streaming-pull: 19262b752ee874eb2ca3b950eb2aef44d5a5267b

‎branches/tswast-patch-1/google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingHandler.java‎

Lines changed: 25 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020

2121
import com.google.cloud.MonitoredResource;
2222
import com.google.cloud.logging.Logging.WriteOption;
23+
import com.google.api.gax.core.ApiFutures;
24+
import com.google.api.gax.core.ApiFutureCallback;
2325
import com.google.common.collect.ImmutableList;
2426
import com.google.common.collect.ImmutableMap;
2527
import java.util.ArrayList;
@@ -106,7 +108,6 @@ public class LoggingHandler extends Handler {
106108

107109
private final LoggingOptions options;
108110
private final WriteOption[] writeOptions;
109-
private List<LogEntry> buffer = new LinkedList<>();
110111
private volatile Logging logging;
111112
private Level flushLevel;
112113
private long flushSize;
@@ -372,22 +373,9 @@ public void publish(LogRecord record) {
372373

373374
try {
374375
LogEntry entry = entryFor(record);
375-
376-
List<LogEntry> flushBuffer = null;
377-
WriteOption[] flushWriteOptions = null;
378-
379-
synchronized (this) {
380-
if (entry != null) {
381-
buffer.add(entry);
382-
}
383-
if (buffer.size() >= flushSize || record.getLevel().intValue() >= flushLevel.intValue()) {
384-
flushBuffer = buffer;
385-
flushWriteOptions = writeOptions;
386-
buffer = new LinkedList<>();
387-
}
376+
if (entry != null) {
377+
write(entry, writeOptions);
388378
}
389-
390-
flush(flushBuffer, flushWriteOptions);
391379
} finally {
392380
inPublishCall.remove();
393381
}
@@ -459,45 +447,38 @@ private static Severity severityFor(Level level) {
459447
* Writes the provided list of log entries to Stackdriver Logging. Override this method to change
460448
* how entries should be written.
461449
*/
462-
void write(List<LogEntry> entries, WriteOption... options) {
450+
void write(LogEntry entry, WriteOption... options) {
451+
List<LogEntry> entryList = Collections.singletonList(entry);
463452
switch (this.synchronicity) {
464453
case SYNC:
465-
getLogging().write(entries, options);
454+
try {
455+
getLogging().write(entryList, options);
456+
} catch (Exception ex) {
457+
reportError(null, ex, ErrorManager.FLUSH_FAILURE);
458+
}
466459
break;
467460
case ASYNC:
468461
default:
469-
getLogging().writeAsync(entries, options);
462+
ApiFutures.addCallback(getLogging().writeAsync(entryList, options), new ApiFutureCallback<Void>() {
463+
@Override
464+
public void onSuccess(Void v) {}
465+
466+
@Override
467+
public void onFailure(Throwable t) {
468+
if (t instanceof Exception) {
469+
reportError(null, (Exception) t, ErrorManager.FLUSH_FAILURE);
470+
} else {
471+
reportError(null, new Exception(t), ErrorManager.FLUSH_FAILURE);
472+
}
473+
}
474+
});
470475
break;
471476
}
472477
}
473478

474479
@Override
475480
public void flush() {
476-
List<LogEntry> flushBuffer;
477-
WriteOption[] flushWriteOptions;
478-
479-
synchronized (this) {
480-
if (buffer.isEmpty()) {
481-
return;
482-
}
483-
flushBuffer = buffer;
484-
flushWriteOptions = writeOptions;
485-
buffer = new LinkedList<>();
486-
}
487-
488-
flush(flushBuffer, flushWriteOptions);
489-
}
490-
491-
private void flush(List<LogEntry> flushBuffer, WriteOption[] flushWriteOptions) {
492-
if (flushBuffer == null) {
493-
return;
494-
}
495-
try {
496-
write(flushBuffer, flushWriteOptions);
497-
} catch (Exception ex) {
498-
// writing can fail but we should not throw an exception, we report the error instead
499-
reportError(null, ex, ErrorManager.FLUSH_FAILURE);
500-
}
481+
// BUG(1795): flush is broken, need support from batching implementation.
501482
}
502483

503484
/**

‎branches/tswast-patch-1/google-cloud-logging/src/test/java/com/google/cloud/logging/LoggingHandlerTest.java‎

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ public void testReportFlushError() {
322322
EasyMock.expect(options.getService()).andReturn(logging);
323323
RuntimeException ex = new RuntimeException();
324324
logging.writeAsync(ImmutableList.of(FINEST_ENTRY), DEFAULT_OPTIONS);
325-
EasyMock.expectLastCall().andThrow(ex);
325+
EasyMock.expectLastCall().andReturn(ApiFutures.immediateFailedFuture(ex));
326326
EasyMock.replay(options, logging);
327327
ErrorManager errorManager = EasyMock.createStrictMock(ErrorManager.class);
328328
errorManager.error(null, ex, ErrorManager.FLUSH_FAILURE);
@@ -356,29 +356,8 @@ public void testReportFormatError() {
356356
EasyMock.verify(errorManager, formatter);
357357
}
358358

359-
@Test
360-
public void testFlushSize() {
361-
EasyMock.expect(options.getProjectId()).andReturn(PROJECT).anyTimes();
362-
EasyMock.expect(options.getService()).andReturn(logging);
363-
logging.writeAsync(
364-
ImmutableList.of(
365-
FINEST_ENTRY, FINER_ENTRY, FINE_ENTRY, CONFIG_ENTRY, INFO_ENTRY, WARNING_ENTRY),
366-
DEFAULT_OPTIONS);
367-
EasyMock.expectLastCall().andReturn(ApiFutures.immediateFuture(null));
368-
EasyMock.replay(options, logging);
369-
LoggingHandler handler = new LoggingHandler(LOG_NAME, options);
370-
handler.setLevel(Level.ALL);
371-
handler.setFlushSize(6);
372-
handler.setFormatter(new TestFormatter());
373-
handler.publish(newLogRecord(Level.FINEST, MESSAGE));
374-
handler.publish(newLogRecord(Level.FINER, MESSAGE));
375-
handler.publish(newLogRecord(Level.FINE, MESSAGE));
376-
handler.publish(newLogRecord(Level.CONFIG, MESSAGE));
377-
handler.publish(newLogRecord(Level.INFO, MESSAGE));
378-
handler.publish(newLogRecord(Level.WARNING, MESSAGE));
379-
}
380-
381-
@Test
359+
// BUG(1795): rewrite this test when flush actually works.
360+
// @Test
382361
public void testFlushLevel() {
383362
EasyMock.expect(options.getProjectId()).andReturn(PROJECT).anyTimes();
384363
EasyMock.expect(options.getService()).andReturn(logging);

0 commit comments

Comments
 (0)