Summary cleanup [0.10]

packages/loader/container-loader/src/container.ts

@@ -881,9 +881,9 @@ export class Container extends EventEmitterWithErrorHandling implements IContain
         // We do not have good correlation ID to match server activity.
         // Add couple IDs here
         this.subLogger.setProperties({
-            SocketClientId: this.clientId,
-            SocketDocumentId: this._deltaManager!.socketDocumentId,
-            SocketPendingClientId: value === ConnectionState.Connecting ? this.pendingClientId : undefined,
+            clientId: this.clientId,
+            socketDocumentId: this._deltaManager!.socketDocumentId,
+            pendingClientId: value === ConnectionState.Connecting ? this.pendingClientId : undefined,
         });
 
         // Log actual event

packages/loader/utils/src/blobs.ts

@@ -148,3 +148,52 @@ export function buildHierarchy(flatTree: git.ITree): ISnapshotTree {
 
     return root;
 }
+
+/**
+ * Basic implementation of a blob ITreeEntry
+ */
+export class BlobTreeEntry implements ITreeEntry {
+    public readonly mode = FileMode.File;
+    public readonly type = TreeEntry[TreeEntry.Blob];
+    public readonly value: IBlob;
+
+    /**
+     * Creates a blob ITreeEntry
+     * @param path - path of entry
+     * @param contents - blob contents
+     * @param encoding - encoding of contents; defaults to utf-8
+     */
+    constructor(public readonly path: string, contents: string, encoding: string = "utf-8") {
+        this.value = { contents, encoding };
+    }
+}
+
+/**
+ * Basic implementation of a commit ITreeEntry
+ */
+export class CommitTreeEntry implements ITreeEntry {
+    public readonly mode = FileMode.Commit;
+    public readonly type = TreeEntry[TreeEntry.Commit];
+
+    /**
+     * Creates a commit ITreeEntry
+     * @param path - path of entry
+     * @param value - commit value
+     */
+    constructor(public readonly path: string, public readonly value: string) {}
+}
+
+/**
+ * Basic implementation of a tree ITreeEntry
+ */
+export class TreeTreeEntry implements ITreeEntry {
+    public readonly mode = FileMode.Directory;
+    public readonly type = TreeEntry[TreeEntry.Tree];
+
+    /**
+     * Creates a tree ITreeEntry
+     * @param path - path of entry
+     * @param value - subtree
+     */
+    constructor(public readonly path: string, public readonly value: ITree) {}
+}

packages/runtime/component-runtime/src/channelContext.ts

@@ -4,14 +4,13 @@
  */
 
 import { ConnectionState } from "@microsoft/fluid-container-definitions";
+import { BlobTreeEntry } from "@microsoft/fluid-core-utils";
 import {
-    FileMode,
     IDocumentStorageService,
     ISequencedDocumentMessage,
     ISnapshotTree,
     ITree,
     MessageType,
-    TreeEntry,
 } from "@microsoft/fluid-protocol-definitions";
 import { IChannel, IEnvelope } from "@microsoft/fluid-runtime-definitions";
 import { ChannelDeltaConnection } from "./channelDeltaConnection";
@@ -59,15 +58,7 @@ export function snapshotChannel(channel: IChannel, baseId: string | null) {
 
     // Add in the object attributes to the returned tree
     const objectAttributes = channel.attributes;
-    snapshot.entries.push({
-        mode: FileMode.File,
-        path: ".attributes",
-        type: TreeEntry[TreeEntry.Blob],
-        value: {
-            contents: JSON.stringify(objectAttributes),
-            encoding: "utf-8",
-        },
-    });
+    snapshot.entries.push(new BlobTreeEntry(".attributes", JSON.stringify(objectAttributes)));
 
     // If baseId exists then the previous snapshot is still valid
     snapshot.id = baseId;

packages/runtime/component-runtime/src/componentRuntime.ts

@@ -14,15 +14,13 @@ import {
     IQuorum,
     ITelemetryLogger,
 } from "@microsoft/fluid-container-definitions";
-import { buildHierarchy, ChildLogger, Deferred, flatten, raiseConnectedEvent } from "@microsoft/fluid-core-utils";
+import { buildHierarchy, ChildLogger, Deferred, flatten, raiseConnectedEvent, TreeTreeEntry } from "@microsoft/fluid-core-utils";
 import {
-    FileMode,
     IDocumentMessage,
     ISequencedDocumentMessage,
     ISnapshotTree,
     ITreeEntry,
     MessageType,
-    TreeEntry,
 } from "@microsoft/fluid-protocol-definitions";
 import {
     IAttachMessage,
@@ -460,12 +458,7 @@ export class ComponentRuntime extends EventEmitter implements IComponentRuntime,
                 const snapshot = await value.snapshot(fullTree);
 
                 // And then store the tree
-                return {
-                    mode: FileMode.Directory,
-                    path: key,
-                    type: TreeEntry[TreeEntry.Tree],
-                    value: snapshot,
-                };
+                return new TreeTreeEntry(key, snapshot);
             }));
 
         return entries;
@@ -484,12 +477,7 @@ export class ComponentRuntime extends EventEmitter implements IComponentRuntime,
                 const snapshot = value.getAttachSnapshot();
 
                 // And then store the tree
-                entries.push({
-                    mode: FileMode.Directory,
-                    path: objectId,
-                    type: TreeEntry[TreeEntry.Tree],
-                    value: snapshot,
-                });
+                entries.push(new TreeTreeEntry(objectId, snapshot));
             }
         }
 

packages/runtime/container-runtime/src/componentContext.ts

@@ -14,6 +14,7 @@ import {
     IQuorum,
 } from "@microsoft/fluid-container-definitions";
 import {
+    BlobTreeEntry,
     Deferred,
     raiseConnectedEvent,
     readAndParse,
@@ -42,7 +43,6 @@ import { EventEmitter } from "events";
 // tslint:disable-next-line:no-submodule-imports
 import * as uuid from "uuid/v4";
 import { ContainerRuntime } from "./containerRuntime";
-import { BlobTreeEntry } from "./utils";
 
 // Snapshot Format Version to be used in component attributes.
 const currentSnapshotFormatVersion = "0.1";

packages/runtime/container-runtime/src/containerRuntime.ts

@@ -20,7 +20,9 @@ import {
     ITelemetryLogger,
 } from "@microsoft/fluid-container-definitions";
 import {
+    BlobTreeEntry,
     buildHierarchy,
+    CommitTreeEntry,
     ComponentSerializer,
     Deferred,
     flatten,
@@ -65,8 +67,8 @@ import { DocumentStorageServiceProxy } from "./documentStorageServiceProxy";
 import { LeaderElector } from "./leaderElection";
 import { Summarizer } from "./summarizer";
 import { SummaryManager } from "./summaryManager";
+import { ISummaryStats, SummaryTreeConverter } from "./summaryTreeConverter";
 import { analyzeTasks } from "./taskAnalyzer";
-import { BlobTreeEntry, CommitTreeEntry, ISummaryStats, SummaryTreeConverter } from "./utils";
 
 interface ISummaryTreeWithStats {
     summaryStats: ISummaryStats;
@@ -563,7 +565,9 @@ export class ContainerRuntime extends EventEmitter implements IHostRuntime, IRun
 
         // Create the SummaryManager and mark the initial state
         this.summaryManager = new SummaryManager(
-            context, this.runtimeOptions.generateSummaries || this.loadedFromSummary);
+            context,
+            this.runtimeOptions.generateSummaries || this.loadedFromSummary,
+            this.logger);
         if (this.context.connectionState === ConnectionState.Connected) {
             this.summaryManager.setConnected(this.context.clientId);
         }

packages/runtime/container-runtime/src/index.ts

@@ -4,4 +4,4 @@
  */
 
 export * from "./containerRuntime";
-export * from "./utils";
+export * from "./summaryTreeConverter";

packages/runtime/container-runtime/src/summarizer.ts

@@ -50,7 +50,7 @@ export interface ISummarizer extends IProvideSummarizer {
      * Stops the summarizer by closing its container and resolving its run promise.
      * @param reason - reason for stopping
      */
-    stop(reason?: string);
+    stop(reason?: string): void;
 }
 
 export class Summarizer implements IComponentLoadable, ISummarizer {

packages/runtime/container-runtime/src/summaryManager.ts

@@ -4,11 +4,10 @@
  */
 
 import { IComponent, IRequest } from "@microsoft/fluid-component-core-interfaces";
-import { IContainerContext } from "@microsoft/fluid-container-definitions";
-import { Heap, IComparer, IHeapNode } from "@microsoft/fluid-core-utils";
+import { IContainerContext, ITelemetryLogger } from "@microsoft/fluid-container-definitions";
+import { ChildLogger, Heap, IComparer, IHeapNode } from "@microsoft/fluid-core-utils";
 import { ISequencedClient } from "@microsoft/fluid-protocol-definitions";
 import { EventEmitter } from "events";
-import { debug } from "./debug";
 import { ISummarizer } from "./summarizer";
 
 interface ITrackedClient {
@@ -34,6 +33,7 @@ export class SummaryManager extends EventEmitter {
     private connected = false;
     private clientId: string;
     private runningSummarizer?: ISummarizer;
+    private readonly logger: ITelemetryLogger;
 
     public get summarizer() {
         return this._summarizer;
@@ -43,9 +43,15 @@ export class SummaryManager extends EventEmitter {
         return this.connected && this.clientId === this.summarizer;
     }
 
-    constructor(private readonly context: IContainerContext, private readonly generateSummaries: boolean) {
+    constructor(
+        private readonly context: IContainerContext,
+        private readonly generateSummaries: boolean,
+        parentLogger: ITelemetryLogger,
+    ) {
         super();
 
+        this.logger = ChildLogger.create(parentLogger, "SummaryManager");
+
         const members = context.quorum.getMembers();
         for (const [clientId, member] of members) {
             this.addHeapNode(clientId, member);
@@ -103,13 +109,18 @@ export class SummaryManager extends EventEmitter {
      */
     private computeSummarizer(force = false) {
         const clientId = this.heap.count() > 0 ? this.heap.peek().value.clientId : undefined;
+
+        // Do not start if we are already the summarizer, unless force is passed.
+        // Force will be passed if we think we are not already summarizing, which might
+        // happen if we are not yet connected the first time through computeSummarizer.
         if (!force && clientId === this._summarizer) {
             return;
         }
 
         this._summarizer = clientId;
         this.emit("summarizer", clientId);
 
+        // do not start if we are not the summarizer or not connected
         if (this._summarizer !== this.clientId || !this.connected) {
             return;
         }
@@ -134,10 +145,10 @@ export class SummaryManager extends EventEmitter {
                 () => {
                     // In the future we will respawn the summarizer - for now we simply stop
                     // this.computeSummarizer(this.connected)
-                    debug("summary generation complete");
+                    this.logger.sendTelemetryEvent({ eventName: "RunningSummarizerCompleted" });
                 },
                 (error) => {
-                    debug("summary generation error", error);
+                    this.logger.sendErrorEvent({ eventName: "RunningSummarizerFailed" }, error);
                 });
         }
 
@@ -146,7 +157,7 @@ export class SummaryManager extends EventEmitter {
     private async createSummarizer() {
         // We have been elected the summarizer. Some day we may be able to summarize with a live document but for
         // now we play it safe and launch a second copy.
-        debug(`${this.clientId} elected summarizer`);
+        this.logger.sendTelemetryEvent({ eventName: "CreatingSummarizer" });
 
         const loader = this.context.loader;
 

packages/runtime/container-runtime/src/utils.ts → packages/runtime/container-runtime/src/summaryTreeConverter.ts

@@ -4,12 +4,10 @@
  */
 
 import {
-    FileMode,
     IBlob,
     ISummaryBlob,
     ISummaryTree,
     ITree,
-    ITreeEntry,
     SummaryObject,
     SummaryTree,
     SummaryType,
@@ -123,27 +121,3 @@ export class SummaryTreeConverter {
         }
     }
 }
-
-export class BlobTreeEntry implements ITreeEntry {
-    public readonly mode = FileMode.File;
-    public readonly type = TreeEntry[TreeEntry.Blob];
-    public readonly value: IBlob;
-
-    constructor(public readonly path: string, contents: string, encoding: string = "utf-8") {
-        this.value = { contents, encoding };
-    }
-}
-
-export class CommitTreeEntry implements ITreeEntry {
-    public readonly mode = FileMode.Commit;
-    public readonly type = TreeEntry[TreeEntry.Commit];
-
-    constructor(public readonly path: string, public readonly value: string) {}
-}
-
-export class TreeTreeEntry implements ITreeEntry {
-    public readonly mode = FileMode.Directory;
-    public readonly type = TreeEntry[TreeEntry.Tree];
-
-    constructor(public readonly path: string, public readonly value: ITree) {}
-}

packages/runtime/container-runtime/src/test/componentContext.spec.ts

@@ -4,11 +4,10 @@
  */
 // tslint:disable: prefer-const
 import { IComponent } from "@microsoft/fluid-component-core-interfaces";
-import { IDocumentStorageService, ISnapshotTree } from "@microsoft/fluid-protocol-definitions";
+import { IBlob, IDocumentStorageService, ISnapshotTree } from "@microsoft/fluid-protocol-definitions";
 import { ComponentFactoryTypes, IComponentContext, IComponentFactory, IComponentRegistry, IComponentRuntime } from "@microsoft/fluid-runtime-definitions";
 import { MockRuntime } from "@microsoft/fluid-test-runtime-utils";
 import * as assert from "assert";
-import { BlobTreeEntry } from "..";
 import { IComponentAttributes, LocalComponentContext, RemotedComponentContext } from "../componentContext";
 import { ContainerRuntime } from "../containerRuntime";
 import { DocumentStorageServiceProxy } from "../documentStorageServiceProxy";
@@ -45,9 +44,9 @@ describe("Component Context Tests", () => {
             localComponentContext.bindRuntime(new MockRuntime());
             const attachMessage = localComponentContext.generateAttachMessage();
 
-            const blob = attachMessage.snapshot.entries[0] as BlobTreeEntry;
+            const blob = attachMessage.snapshot.entries[0].value as IBlob;
 
-            const contents = JSON.parse(blob.value.contents) as IComponentAttributes;
+            const contents = JSON.parse(blob.contents) as IComponentAttributes;
             const componentAttributes: IComponentAttributes = {
                 pkg: JSON.stringify(["TestComponent1"]),
                 snapshotFormatVersion: "0.1",
@@ -93,8 +92,8 @@ describe("Component Context Tests", () => {
             localComponentContext.bindRuntime(new MockRuntime());
 
             const attachMessage = localComponentContext.generateAttachMessage();
-            const blob = attachMessage.snapshot.entries[0] as BlobTreeEntry;
-            const contents = JSON.parse(blob.value.contents) as IComponentAttributes;
+            const blob = attachMessage.snapshot.entries[0].value as IBlob;
+            const contents = JSON.parse(blob.contents) as IComponentAttributes;
             const componentAttributes: IComponentAttributes = {
                 pkg: JSON.stringify(["TestComp", "SubComp"]),
                 snapshotFormatVersion: "0.1",
@@ -149,9 +148,9 @@ describe("Component Context Tests", () => {
                 new DocumentStorageServiceProxy(storage, blobCache),
                 scope);
             const snapshot = await remotedComponentContext.snapshot(true);
-            const blob = snapshot.entries[0] as BlobTreeEntry;
+            const blob = snapshot.entries[0].value as IBlob;
 
-            const contents = JSON.parse(blob.value.contents) as IComponentAttributes;
+            const contents = JSON.parse(blob.contents) as IComponentAttributes;
             assert.equal(contents.pkg, componentAttributes.pkg, "Remote Component package does not match.");
             assert.equal(contents.snapshotFormatVersion, componentAttributes.snapshotFormatVersion, "Remote Component snapshot version does not match.");
         });
@@ -176,9 +175,9 @@ describe("Component Context Tests", () => {
                 new DocumentStorageServiceProxy(storage, blobCache),
                 scope);
             const snapshot = await remotedComponentContext.snapshot(true);
-            const blob = snapshot.entries[0] as BlobTreeEntry;
+            const blob = snapshot.entries[0].value as IBlob;
 
-            const contents = JSON.parse(blob.value.contents) as IComponentAttributes;
+            const contents = JSON.parse(blob.contents) as IComponentAttributes;
             assert.equal(contents.pkg, JSON.stringify([componentAttributes.pkg]), "Remote Component package does not match.");
             assert.equal(contents.snapshotFormatVersion, "0.1", "Remote Component snapshot version does not match.");
         });